Implement CTAS for columnstore #75

nbiscaro · 2025-01-07T22:54:45Z

Implements #26

Adding support for CREATE TABLE AS SELECT for columnstore tables.

This enhancement allows users to seamlessly execute CTAS (CREATE TABLE AS SELECT) queries between columnstore and any existing heap or columnstore tables. For example:

CREATE TABLE columnstore_table USING columnstore AS SELECT * FROM heap_or_columnstore_table;

nbiscaro · 2025-01-07T22:59:59Z

src/pgduckdb/pgduckdb_ddl.cpp

@@ -57,8 +58,8 @@ DuckdbHandleDDL(Node *parsetree) {
 	if (IsA(parsetree, CreateTableAsStmt)) {
 		auto stmt = castNode(CreateTableAsStmt, parsetree);
 		char *access_method = stmt->into->accessMethod ? stmt->into->accessMethod : default_table_access_method;
-		if (strcmp(access_method, "duckdb") != 0) {
-			/* not a duckdb table, so don't mess with the query */
+		if (strcmp(access_method, "duckdb") != 0 && strcmp(access_method, "columnstore") != 0) {


This is necessary to allow stmt->into->skipData = true;. We implement our CTAS logic after prev_process_utility_hook which creates our table but "skips" inserting any data as a result. This is equivalent to running CREATE TABLE AS ... WITH NO_DATA manually, although it feels a little more natural to use existing codepaths.

I’d prefer not to mix code into DuckdbHandleDDL(). Instead, copy the following code

ctas_skip_data = stmt->into->skipData; stmt->into->skipData = true;

to wherever it's needed

DuckdbHandleDDL() is specifically for DuckDB temp tables, which we don’t enable at all. Ideally, we should comment out the call to DuckdbHandleDDL() in DuckdbUtilityHook_Cpp()

Given this comment I don't think we need to set ctas_skip_data. Otherwise, sounds good.

But then how do you handle CREATE TABLE AS ... WITH NO DATA?

dpxcc · 2025-01-08T00:54:13Z

src/pgduckdb/pgduckdb_ddl.cpp

@@ -57,8 +58,8 @@ DuckdbHandleDDL(Node *parsetree) {
 	if (IsA(parsetree, CreateTableAsStmt)) {
 		auto stmt = castNode(CreateTableAsStmt, parsetree);
 		char *access_method = stmt->into->accessMethod ? stmt->into->accessMethod : default_table_access_method;
-		if (strcmp(access_method, "duckdb") != 0) {
-			/* not a duckdb table, so don't mess with the query */
+		if (strcmp(access_method, "duckdb") != 0 && strcmp(access_method, "columnstore") != 0) {


I’d prefer not to mix code into DuckdbHandleDDL(). Instead, copy the following code

ctas_skip_data = stmt->into->skipData; stmt->into->skipData = true;

to wherever it's needed

DuckdbHandleDDL() is specifically for DuckDB temp tables, which we don’t enable at all. Ideally, we should comment out the call to DuckdbHandleDDL() in DuckdbUtilityHook_Cpp()

dpxcc · 2025-01-08T00:57:47Z

test/sql/create_table_as.sql

+
+CREATE TABLE t USING columnstore AS SELECT * FROM c;
+SELECT * FROM t;
+DROP TABLE t;


Can we also add a test for CTAS from columnstore to Postgres heap table? It seems that it's also working duckdb/pg_duckdb#520

dpxcc · 2025-01-08T05:35:51Z

src/pgduckdb/pgduckdb_ddl.cpp

+// 		if (strcmp(access_method, "duckdb") != 0 && strcmp(access_method, "columnstore") != 0) {
+// 			/* not a duckdb or columnstore table, so don't mess with the query */


should revert the previous changes here

dpxcc · 2025-01-08T05:38:44Z

test/sql/create_table_as.sql

@@ -0,0 +1,26 @@
+LOAD 'pg_mooncake';


this is no longer needed

dpxcc · 2025-01-09T00:10:36Z

src/pgduckdb/pgduckdb_ddl.cpp

+		}
+
+		auto *stmt = (CreateTableAsStmt *)parsetree;
+		if (!stmt->into->accessMethod) {


It seems that when stmt->into->accessMethod is unset, PG will use default table access method, and we need to make sure it's not columnstore

dpxcc · 2025-01-09T00:12:22Z

src/pgduckdb/pgduckdb_ddl.cpp

 	prev_process_utility_hook(pstmt, query_string, read_only_tree, context, params, query_env, dest, qc);

+	if (auto *ctas_stmt = get_columnstore_ctas_stmt()) {
+		auto *ctas_query = (Query *)ctas_stmt->query;


It’s preferable to query DuckDB directly rather than using the PostgreSQL interface, as the latter internally translates and issues queries to DuckDB

dpxcc · 2025-01-09T00:15:29Z

src/pgduckdb/pgduckdb_ddl.cpp

+			elog(ERROR, "Failed to connect to SPI for INSERT INTO execution.");
+		}
+
+		int ret = SPI_execute(insert_query.c_str(), false, 0);


Is it possible that CREATE TABLE AS ... WITH NO_DATA succeeds, but INSERT ... SELECT fails? We need to ensure that these two queries are performed atomically

Use `CREATE TABLE AS ... WITH NO DATA` followed by `INSERT ... SELECT`

dpxcc · 2025-01-09T13:00:33Z

@nbiscaro
I've cleaned up your implementation: 55f5667
It addressed all comments and I manually verified that CREATE TABLE AS ... WITH NO DATA and INSERT ... SELECT are atomic
Let me know if you have any comments, otherwise I'm planning to merge it later today so that we can release v0.1 this Friday

dpxcc · 2025-01-09T23:46:37Z

pushed 55f5667

nbiscaro · 2025-01-09T23:50:41Z

Thanks for cleaning this up. Looks great!

Use `CREATE TABLE AS ... WITH NO DATA` followed by `INSERT ... SELECT`

nbiscaro added 3 commits January 7, 2025 22:05

mv test files from unsupported

f561540

add new CTAS tests

f69103f

add CTAS for columnstore

482dcd9

nbiscaro commented Jan 7, 2025

View reviewed changes

dpxcc reviewed Jan 8, 2025

View reviewed changes

nbiscaro added 3 commits January 8, 2025 03:41

set skip_data directly

e07e70a

add test for CTAS from columnstore to heap

b5c3db1

comment out DuckdbHandleDDL

32152ac

dpxcc reviewed Jan 9, 2025

View reviewed changes

dpxcc pushed a commit that referenced this pull request Jan 9, 2025

Implement CTAS (#75)

55f5667

Use `CREATE TABLE AS ... WITH NO DATA` followed by `INSERT ... SELECT`

dpxcc closed this Jan 9, 2025

dpxcc pushed a commit that referenced this pull request Jan 10, 2025

Implement CTAS (#75)

e0027c3

Use `CREATE TABLE AS ... WITH NO DATA` followed by `INSERT ... SELECT`

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement CTAS for columnstore #75

Implement CTAS for columnstore #75

nbiscaro commented Jan 7, 2025 •

edited

Loading

nbiscaro Jan 7, 2025

dpxcc Jan 8, 2025

nbiscaro Jan 8, 2025

dpxcc Jan 9, 2025

dpxcc Jan 8, 2025

dpxcc Jan 8, 2025

dpxcc Jan 8, 2025

dpxcc Jan 8, 2025

dpxcc Jan 9, 2025

dpxcc Jan 9, 2025

dpxcc Jan 9, 2025

dpxcc commented Jan 9, 2025

dpxcc commented Jan 9, 2025

nbiscaro commented Jan 9, 2025

		// if (strcmp(access_method, "duckdb") != 0 && strcmp(access_method, "columnstore") != 0) {
		// /* not a duckdb or columnstore table, so don't mess with the query */

Implement CTAS for columnstore #75

Implement CTAS for columnstore #75

Conversation

nbiscaro commented Jan 7, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dpxcc commented Jan 9, 2025

dpxcc commented Jan 9, 2025

nbiscaro commented Jan 9, 2025

nbiscaro commented Jan 7, 2025 •

edited

Loading