fix bug where caching would be inabled when it shouldn't

Author: samansmink

Makefile

@@ -43,6 +43,7 @@ clean:
 	rm -rf build
 	rm -rf testext
 	cd duckdb && make clean
+	cd duckdb/tools/nodejs && make clean
 
 # Main build
 debug:
@@ -68,16 +69,16 @@ test_release: release
 	./build/release/test/unittest "$(PROJ_DIR)test/*"
 
 test_debug: debug
-	./build/release/test/unittest "$(PROJ_DIR)test/*"
+	./build/debug/test/unittest "$(PROJ_DIR)test/*"
 
 # Client tests
 DEBUG_EXT_PATH='$(PROJ_DIR)build/debug/extension/arrow/arrow.duckdb_extension'
-RELEASE_EXT_PATH='$(PROJ_DIR)build/debug/extension/arrow/arrow.duckdb_extension'
+RELEASE_EXT_PATH='$(PROJ_DIR)build/release/extension/arrow/arrow.duckdb_extension'
 test_js: test_debug_js
 test_debug_js: debug_js
 	cd duckdb/tools/nodejs && ARROW_EXTENSION_BINARY_PATH=$(DEBUG_EXT_PATH) npm run test-path -- "../../../test/nodejs/**/*.js"
 test_release_js: release_js
-	cd duckdb/tools/nodejs && ARROW_EXTENSION_BINARY_PATH=$(DEBUG_EXT_PATH) npm run test-path -- "../../../test/nodejs/**/*.js"
+	cd duckdb/tools/nodejs && ARROW_EXTENSION_BINARY_PATH=$(RELEASE_EXT_PATH) npm run test-path -- "../../../test/nodejs/**/*.js"
 
 format:
 	find src/ -iname *.hpp -o -iname *.cpp | xargs clang-format --sort-includes=0 -style=file -i

duckdb

@@ -8,12 +8,11 @@ TableFunction ArrowIPCTableFunction::GetFunction() {
 
     TableFunction scan_arrow_ipc_func(
             "scan_arrow_ipc", {LogicalType::LIST(LogicalType::STRUCT(make_buffer_struct_children))},
-            ArrowIPCTableFunction::ArrowScanFunction, ArrowIPCTableFunction::ArrowScanBind,
-            ArrowTableFunction::ArrowScanInitGlobal, ArrowTableFunction::ArrowScanInitLocal);
+        ArrowIPCTableFunction::ArrowScanFunction, ArrowIPCTableFunction::ArrowScanBind,
+        ArrowTableFunction::ArrowScanInitGlobal, ArrowTableFunction::ArrowScanInitLocal);
 
     scan_arrow_ipc_func.cardinality = ArrowTableFunction::ArrowScanCardinality;
-    // FIXME this currently does not work yet
-//    scan_arrow_ipc_func.get_batch_index = ArrowTableFunction::ArrowGetBatchIndex;
+    scan_arrow_ipc_func.get_batch_index = nullptr; // TODO implement
     scan_arrow_ipc_func.projection_pushdown = true;
     scan_arrow_ipc_func.filter_pushdown = false;
 
@@ -80,7 +79,7 @@ unique_ptr <FunctionData> ArrowIPCTableFunction::ArrowScanBind(ClientContext &co
     return std::move(res);
 }
 
-// TODO: cleanup: only difference is the ArrowToDuckDB call
+// Same as regular arrow scan, except ArrowToDuckDB call TODO: refactor to allow nicely overriding this
 void ArrowIPCTableFunction::ArrowScanFunction(ClientContext &context, TableFunctionInput &data_p, DataChunk &output) {
     if (!data_p.local_state) {
         return;
@@ -90,16 +89,23 @@ void ArrowIPCTableFunction::ArrowScanFunction(ClientContext &context, TableFunct
     auto &global_state = data_p.global_state->Cast<ArrowScanGlobalState>();
 
     //! Out of tuples in this chunk
-    if (state.chunk_offset >= (idx_t) state.chunk->arrow_array.length) {
-        if (!ArrowTableFunction::ArrowScanParallelStateNext(context, data_p.bind_data.get(), state, global_state)) {
+    if (state.chunk_offset >= (idx_t)state.chunk->arrow_array.length) {
+        if (!ArrowScanParallelStateNext(context, data_p.bind_data.get(), state, global_state)) {
             return;
         }
     }
-    int64_t output_size =
-            MinValue<int64_t>(STANDARD_VECTOR_SIZE, state.chunk->arrow_array.length - state.chunk_offset);
+    int64_t output_size = MinValue<int64_t>(STANDARD_VECTOR_SIZE, state.chunk->arrow_array.length - state.chunk_offset);
     data.lines_read += output_size;
-    output.SetCardinality(output_size);
-    ArrowTableFunction::ArrowToDuckDB(state, data.arrow_table.GetColumns(), output, data.lines_read - output_size, false);
+    if (global_state.CanRemoveFilterColumns()) {
+        state.all_columns.Reset();
+        state.all_columns.SetCardinality(output_size);
+        ArrowToDuckDB(state, data.arrow_table.GetColumns(), state.all_columns, data.lines_read - output_size, false);
+        output.ReferenceColumns(state.all_columns, global_state.projection_ids);
+    } else {
+        output.SetCardinality(output_size);
+        ArrowToDuckDB(state, data.arrow_table.GetColumns(), output, data.lines_read - output_size, false);
+    }
+
     output.Verify();
     state.chunk_offset += output.size();
 }

src/arrow_scan_ipc.cpp

@@ -87,7 +87,7 @@ OperatorResultType ToArrowIPCFunction::Function(ExecutionContext &context, Table
 
     bool sending_schema = false;
 
-    bool caching_disabled = context.pipeline && !context.pipeline->GetSink();
+    bool caching_disabled = !PhysicalOperator::OperatorCachingAllowed(context);
 
     if (!local_state.checked_schema) {
         if (!global_state.sent_schema) {

src/arrow_to_ipc.cpp

@@ -9,4 +9,16 @@
 require arrow
 
 statement ok
-SELECT * FROM to_arrow_ipc((SELECT 'Its working!'));
+SELECT * FROM to_arrow_ipc((SELECT 'Its working!'));
+
+# Test operator caching behaviour is sane
+statement ok
+create table data as select * from range(0,2000) tbl(col)
+
+statement ok
+WITH data_union AS (
+    SELECT * FROM data
+    UNION ALL
+    SELECT * FROM data
+)
+SELECT * FROM to_arrow_ipc((SELECT * FROM data_union ORDER BY col));