Skip to content
This repository was archived by the owner on May 9, 2024. It is now read-only.

Commit f8e5004

Browse files
committed
Use raw pointers for schema provider and config when passed to Calcite
Expect higher layers to handle lifetime management of these objects
1 parent 8f22a84 commit f8e5004

File tree

9 files changed

+27
-23
lines changed

9 files changed

+27
-23
lines changed

omniscidb/Calcite/CalciteJNI.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -201,8 +201,8 @@ class CalciteJNI {
201201

202202
std::string process(const std::string& db_name,
203203
const std::string& sql_string,
204-
SchemaProviderPtr schema_provider,
205-
ConfigPtr config,
204+
SchemaProvider* schema_provider,
205+
Config* config,
206206
const std::vector<FilterPushDownInfo>& filter_push_down_info,
207207
const bool legacy_syntax,
208208
const bool is_explain,
@@ -575,8 +575,8 @@ CalciteMgr::~CalciteMgr() {
575575
std::string CalciteMgr::process(
576576
const std::string& db_name,
577577
const std::string& sql_string,
578-
SchemaProviderPtr schema_provider,
579-
ConfigPtr config,
578+
SchemaProvider* schema_provider,
579+
Config* config,
580580
const std::vector<FilterPushDownInfo>& filter_push_down_info,
581581
const bool legacy_syntax,
582582
const bool is_explain,

omniscidb/Calcite/CalciteJNI.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,8 @@ class CalciteMgr {
5353

5454
std::string process(const std::string& db_name,
5555
const std::string& sql_string,
56-
SchemaProviderPtr schema_provider,
57-
ConfigPtr config,
56+
SchemaProvider* schema_provider,
57+
Config* config,
5858
const std::vector<FilterPushDownInfo>& filter_push_down_info = {},
5959
const bool legacy_syntax = false,
6060
const bool is_explain = false,

omniscidb/Calcite/SchemaJson.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ int getCalciteScale(const hdk::ir::Type* type) {
183183

184184
} // namespace
185185

186-
std::string schema_to_json(SchemaProviderPtr schema_provider) {
186+
std::string schema_to_json(SchemaProvider* schema_provider) {
187187
auto dbs = schema_provider->listDatabases();
188188
if (dbs.empty()) {
189189
return "{}";

omniscidb/Calcite/SchemaJson.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,4 +14,4 @@
1414

1515
#include "SchemaMgr/SchemaProvider.h"
1616

17-
std::string schema_to_json(SchemaProviderPtr schema_provider);
17+
std::string schema_to_json(SchemaProvider* schema_provider);

omniscidb/Tests/ArrowSQLRunner/RelAlgCache.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ std::string RelAlgCache::process(
6161

6262
std::string schema_json;
6363
if (!use_cache_.empty() || !build_cache_.empty()) {
64-
schema_json = schema_to_json(schema_provider_);
64+
schema_json = schema_to_json(schema_provider_.get());
6565
}
6666

6767
if (!use_cache_.empty()) {
@@ -89,8 +89,8 @@ std::string RelAlgCache::process(
8989

9090
auto ra = calcite_->process(db_name,
9191
sql_string,
92-
schema_provider_,
93-
config_,
92+
schema_provider_.get(),
93+
config_.get(),
9494
filter_push_down_info,
9595
legacy_syntax,
9696
is_explain,

omniscidb/Tests/NoCatalogSqlTest.cpp

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -162,12 +162,14 @@ class NoCatalogSqlTest : public ::testing::Test {
162162
}
163163

164164
ExecutionResult runSqlQuery(const std::string& sql, Executor* executor) {
165-
const auto query_ra = calcite_->process("test_db", sql, schema_provider_, config_);
165+
const auto query_ra =
166+
calcite_->process("test_db", sql, schema_provider_.get(), config_.get());
166167
return runRAQuery(query_ra, executor);
167168
}
168169

169170
RelAlgExecutor getExecutor(const std::string& sql) {
170-
const auto query_ra = calcite_->process("test_db", sql, schema_provider_, config_);
171+
const auto query_ra =
172+
calcite_->process("test_db", sql, schema_provider_.get(), config_.get());
171173
auto dag = std::make_unique<RelAlgDagBuilder>(
172174
query_ra, TEST_DB_ID, schema_provider_, config_);
173175
return RelAlgExecutor(executor_.get(), schema_provider_, std::move(dag));
@@ -234,7 +236,7 @@ TEST_F(NoCatalogSqlTest, GroupBySingleColumn) {
234236
}
235237

236238
TEST_F(NoCatalogSqlTest, MultipleCalciteMultipleThreads) {
237-
constexpr size_t TEST_NTHREADS = 1000;
239+
constexpr size_t TEST_NTHREADS = 100;
238240
std::vector<ExecutionResult> res;
239241
std::vector<std::future<void>> threads;
240242
res.resize(TEST_NTHREADS);
@@ -252,8 +254,8 @@ TEST_F(NoCatalogSqlTest, MultipleCalciteMultipleThreads) {
252254
&res,
253255
&executors,
254256
calcite,
255-
schema_provider = schema_provider_,
256-
config = config_]() {
257+
schema_provider = schema_provider_.get(),
258+
config = config_.get()]() {
257259
auto query_ra = calcite->process(
258260
"test_db",
259261
"SELECT col_bi + " + std::to_string(i) + " FROM test1;",
@@ -279,7 +281,8 @@ TEST(CalciteReinitTest, SingleThread) {
279281
auto config = std::make_shared<Config>();
280282
for (int i = 0; i < 10; ++i) {
281283
auto calcite = CalciteMgr::get();
282-
auto query_ra = calcite->process("test_db", "SELECT 1;", schema_provider, config);
284+
auto query_ra =
285+
calcite->process("test_db", "SELECT 1;", schema_provider.get(), config.get());
283286
CHECK(query_ra.find("LogicalValues") != std::string::npos) << query_ra;
284287
}
285288
}
@@ -290,7 +293,8 @@ TEST(CalciteReinitTest, MultipleThreads) {
290293
for (int i = 0; i < 10; ++i) {
291294
auto f = std::async(std::launch::async, [schema_provider, config]() {
292295
auto calcite = CalciteMgr::get();
293-
auto query_ra = calcite->process("test_db", "SELECT 1;", schema_provider, config);
296+
auto query_ra =
297+
calcite->process("test_db", "SELECT 1;", schema_provider.get(), config.get());
294298
CHECK(query_ra.find("LogicalValues") != std::string::npos) << query_ra;
295299
});
296300
f.wait();

python/pyhdk/_sql.pxd

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ from libcpp.string cimport string
99
from libcpp.vector cimport vector
1010

1111
from pyhdk._common cimport CConfig
12-
from pyhdk._storage cimport CSchemaProviderPtr, CDataProvider, CDataMgr, CBufferProvider
12+
from pyhdk._storage cimport CSchemaProvider, CSchemaProviderPtr, CDataProvider, CDataMgr, CBufferProvider
1313
from pyhdk._execute cimport CExecutor, CResultSetPtr, CCompilationOptions, CExecutionOptions, CTargetMetaInfo
1414

1515
cdef extern from "omniscidb/QueryEngine/ExtensionFunctionsWhitelist.h":
@@ -33,7 +33,7 @@ cdef extern from "omniscidb/Calcite/CalciteJNI.h":
3333
@staticmethod
3434
CalciteMgr* get(const string&, size_t);
3535

36-
string process(const string&, const string&, CSchemaProviderPtr, shared_ptr[CConfig], const vector[FilterPushDownInfo]&, bool, bool, bool) except +
36+
string process(const string&, const string&, CSchemaProvider*, CConfig*, const vector[FilterPushDownInfo]&, bool, bool, bool) except +
3737

3838
string getExtensionFunctionWhitelist()
3939
string getUserDefinedFunctionWhitelist()

python/pyhdk/_sql.pyx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ cdef class Calcite:
4040
cdef bool legacy_syntax = kwargs.get("legacy_syntax", False)
4141
cdef bool is_explain = kwargs.get("is_explain", False)
4242
cdef bool is_view_optimize = kwargs.get("is_view_optimize", False)
43-
return self.calcite.process(db_name, sql, self.schema_provider, self.config, filter_push_down_info, legacy_syntax, is_explain, is_view_optimize)
43+
return self.calcite.process(db_name, sql, self.schema_provider.get(), self.config.get(), filter_push_down_info, legacy_syntax, is_explain, is_view_optimize)
4444

4545
cdef class ExecutionResult:
4646
def row_count(self):

src/HDK.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,8 @@ ExecutionResult HDK::query(const std::string& sql, const bool is_explain) {
4040
CHECK(internal_->calcite);
4141
auto ra = internal_->calcite->process(internal_->db_name,
4242
sql,
43-
internal_->storage,
44-
internal_->config,
43+
internal_->storage.get(),
44+
internal_->config.get(),
4545
{},
4646
/*legacy_syntax=*/true);
4747

0 commit comments

Comments
 (0)