Adding CTE Scan Support #885
Conversation
CTE Query Optimizer Nodes
Adding child property deriver and input column deriver for CteScan.
Gautamj/new table test
…lso one small error in Get Output of non leader node
…ma - complicated code
* Fixed some clang-tidy errors * Clang error for CTE parser. * clang fixes * Small bug * Changing binder test for clang. * Changed var name in storage_interface_test.cpp for clang. Co-authored-by: gautam20197 <gautam20197@gmail.com> Co-authored-by: rohanaggarwal <aggarwal.rohan@yahoo.co.in>
| TERRIER_ASSERT(key_size > 0, "KeySize must be greater than 0"); | ||
| // TERRIER_ASSERT(key_size > 0, "KeySize must be greater than 0"); |
There was a problem hiding this comment.
Is this assertion still necessary here? I think it can be modified or just removed.
There was a problem hiding this comment.
Fixed in latest commit, We have to discuss this more with william as this assert is not entirely correct.
| // Conversion of a 32-bit integer into a non-nullable SQL Integer value | ||
| case CastKind::IntToSqlInt: | ||
| return "IntToSqlInt"; | ||
|
|
||
| // Conversion of a 32-bit integer into a non-nullable SQL Decimal value | ||
| case CastKind::IntToSqlDecimal: | ||
| return "IntToSqlDecimal"; | ||
|
|
||
| // Conversion of a SQL boolean value (potentially nullable) into a primitive | ||
| // boolean value | ||
| case CastKind::SqlBoolToBool: | ||
| return "SqlBoolToBool"; | ||
|
|
||
| // Conversion of a primitive boolean into a SQL boolean | ||
| case CastKind::BoolToSqlBool: | ||
| return "BoolToSqlBool"; | ||
|
|
||
| // A cast between integral types (i.e., 8-bit, 16-bit, 32-bit, or 64-bit | ||
| // numbers), excluding to boolean! Boils down to a bitcast, a truncation: | ||
| // a sign-extension, or a zero-extension. The same as in C/C++. | ||
| case CastKind::IntegralCast: | ||
| return "IntegralCast"; | ||
|
|
||
| // An integer to float cast. Only allows widening. | ||
| case CastKind::IntToFloat: | ||
| return "IntToFloat"; | ||
|
|
||
| // A float to integer cast. Only allows widening. | ||
| case CastKind::FloatToInt: | ||
| return "FloatToInt"; | ||
|
|
||
| // A simple bit cast reinterpretation | ||
| case CastKind::BitCast: | ||
| return "BitCast"; | ||
|
|
||
| // 64 bit float To Sql Real | ||
| case CastKind::FloatToSqlReal: | ||
| return "FloatToSqlReal"; | ||
|
|
||
| // Conversion of a SQL timestamp value (potentially nullable) into a primitive timestamp value | ||
| case CastKind::SqlTimestampToTimestamp: | ||
| return "SqlTimestampToTimestamp"; | ||
|
|
||
| // Conversion of a primitive timestamp valueinto a SQL timestamp | ||
| case CastKind::TimestampToSqlTimestamp: | ||
| return "TimestampToSqlTimestamp"; | ||
|
|
||
| // Convert a SQL integer into a SQL real | ||
| case CastKind::SqlIntToSqlReal: | ||
| return "SqlIntToSqlReal"; |
There was a problem hiding this comment.
I think the comments here should have the same indent.
There was a problem hiding this comment.
Fixed in latest commit
| // Use std::map to effectively sort OIDs by their corresponding ID | ||
| std::map<storage::col_id_t, catalog::col_oid_t> inverse_map; | ||
|
|
||
| // Notice the change in the inverse map argument different from sql_table get projection map function | ||
| for (auto col_oid : col_oids_) inverse_map[col_oid_to_id[col_oid]] = col_oid; | ||
|
|
||
| // Populate the projection map using the in-order iterator on std::map | ||
| uint16_t i = 0; | ||
| for (auto &iter : inverse_map) projection_map_[iter.second] = i++; |
There was a problem hiding this comment.
Can we use std::sort here? From my own test of sorting 100,000 std::pair<int, int>, std::sort is 10%~20% faster than using std::map.
There was a problem hiding this comment.
It doesnt matter as you wont have 100,000 columns in a table. Also this is done to have similar api as SQL Table so that in future when it is replaced by a temp memory space, its easier to change.
Codecov Report
@@ Coverage Diff @@
## master #885 +/- ##
==========================================
- Coverage 83.48% 82.20% -1.28%
==========================================
Files 564 571 +7
Lines 40443 37315 -3128
==========================================
- Hits 33762 30674 -3088
+ Misses 6681 6641 -40
Continue to review full report at Codecov.
|
| @@ -2606,7 +2739,7 @@ void Sema::CheckBuiltinCall(ast::CallExpr *call) { | |||
| break; | |||
| } | |||
| default: { | |||
| UNREACHABLE("Unhandled builtin!"); | |||
There was a problem hiding this comment.
Why was this taken out?
There was a problem hiding this comment.
i believe this is a mistake
|
|
||
| /** | ||
| * Return the iterator that will scan the temp table | ||
| */ |
There was a problem hiding this comment.
Extra documentation here?
|
|
||
| CteScanIterator::CteScanIterator(terrier::execution::exec::ExecutionContext *exec_ctx, uint32_t *schema_cols_type, | ||
| uint32_t num_schema_cols) | ||
| : exec_ctx_(exec_ctx), cte_table_oid_(static_cast<catalog::table_oid_t>(999)), table_redo_(nullptr) { |
There was a problem hiding this comment.
Magic number? Can you make this a macro?
| // Create cte_table | ||
| uint32_t cte_table_col_type[1] = {4}; // {INTEGER} | ||
|
|
||
| auto cte_scan = new terrier::execution::sql::CteScanIterator(exec_ctx_.get(), cte_table_col_type, 1); |
There was a problem hiding this comment.
allocate these on the stack instead
apavlo
added
the
do-not-merge
|
Closed to reduce Jenkins contention |
No description provided.