Skip to content

Commit

Permalink
Add fallback type (#40)
Browse files Browse the repository at this point in the history
This PR adds a fallback for unsupported types.

If the column with this type is scanned from (postgres -> DuckDB) we'll
throw an error.
This makes projection pushdown work even when the table contains
unsupported columns as long as they aren't referenced.

This does not add support for the other way around, if a DuckDB type is
produced that does not have a corresponding Postgres type, this PR does
not address that issue.
  • Loading branch information
Tishj authored Jun 12, 2024
1 parent 47f10b0 commit b7a1be7
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 17 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/build_and_test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ jobs:
git clone --branch ${{ matrix.version }} --single-branch --depth 1 https://github.com/postgres/postgres.git
pushd postgres
git branch
./configure --prefix=$PWD/inst/ --enable-cassert --enable-debug --with-openssl
./configure --prefix=$PWD/inst/ --enable-cassert --enable-debug --with-openssl --with-icu --with-libxml
make -j8 install
- name: Build and test pg_quack extension
Expand Down
4 changes: 4 additions & 0 deletions src/quack_duckdb_connection.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include "duckdb.hpp"
#include "duckdb/parser/parsed_data/create_table_function_info.hpp"
#include "duckdb/main/extension_util.hpp"

#include "quack/quack_duckdb_connection.hpp"
#include "quack/quack_heap_scan.hpp"
Expand All @@ -26,6 +27,7 @@ quack_create_duckdb_connection(List *tables, List *neededColumns, const char *qu
duckdb::make_uniq_base<duckdb::ReplacementScanData, quack::PostgresHeapReplacementScanData>(
tables, neededColumns, query));


auto connection = duckdb::make_uniq<duckdb::Connection>(*db);

// Add the postgres_scan inserted by the replacement scan
Expand All @@ -35,6 +37,8 @@ quack_create_duckdb_connection(List *tables, List *neededColumns, const char *qu

auto &catalog = duckdb::Catalog::GetSystemCatalog(context);
context.transaction.BeginTransaction();
auto &instance = *db->instance;
duckdb::ExtensionUtil::RegisterType(instance, "UnsupportedPostgresType", duckdb::LogicalTypeId::VARCHAR);
catalog.CreateTableFunction(context, &heap_scan_info);
context.transaction.Commit();

Expand Down
27 changes: 14 additions & 13 deletions src/quack_planner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,24 +40,25 @@ quack_create_plan(Query *parse, const char *query) {
auto &column = preparedResultTypes[i];
Oid postgresColumnOid = quack::GetPostgresDuckDBType(column);

if (OidIsValid(postgresColumnOid)) {
HeapTuple tp;
Form_pg_type typtup;
if (!OidIsValid(postgresColumnOid)) {
elog(ERROR, "Could not convert DuckDB to Postgres type, likely because the postgres->duckdb conversion was not supported");
}
HeapTuple tp;
Form_pg_type typtup;

tp = SearchSysCache1(TYPEOID, ObjectIdGetDatum(postgresColumnOid));
if (!HeapTupleIsValid(tp))
elog(ERROR, "cache lookup failed for type %u", postgresColumnOid);
tp = SearchSysCache1(TYPEOID, ObjectIdGetDatum(postgresColumnOid));
if (!HeapTupleIsValid(tp))
elog(ERROR, "cache lookup failed for type %u", postgresColumnOid);

typtup = (Form_pg_type)GETSTRUCT(tp);
typtup = (Form_pg_type)GETSTRUCT(tp);

Var *var = makeVar(INDEX_VAR, i + 1, postgresColumnOid, typtup->typtypmod, typtup->typcollation, 0);
Var *var = makeVar(INDEX_VAR, i + 1, postgresColumnOid, typtup->typtypmod, typtup->typcollation, 0);

quackNode->custom_scan_tlist =
lappend(quackNode->custom_scan_tlist,
makeTargetEntry((Expr *)var, i + 1, (char *)preparedQuery->GetNames()[i].c_str(), false));
quackNode->custom_scan_tlist =
lappend(quackNode->custom_scan_tlist,
makeTargetEntry((Expr *)var, i + 1, (char *)preparedQuery->GetNames()[i].c_str(), false));

ReleaseSysCache(tp);
}
ReleaseSysCache(tp);
}

quackNode->custom_private = list_make2(duckdbConnection.release(), preparedQuery.release());
Expand Down
5 changes: 2 additions & 3 deletions src/quack_types.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,7 @@ ConvertPostgresToDuckColumnType(Form_pg_attribute &attribute) {
return duck_type;
}
default:
elog(ERROR, "(DuckDB/ConvertPostgresToDuckColumnType) Unsupported quack type: %d", type);
return duckdb::LogicalType::USER("UnsupportedPostgresType");
}
}

Expand Down Expand Up @@ -568,8 +568,7 @@ GetPostgresDuckDBType(duckdb::LogicalType type) {
}
}
default: {
elog(ERROR, "(DuckDB/GetPostgresDuckDBType) Unsupported quack type: %s", type.ToString().c_str());
break;
return InvalidOid;
}
}
}
Expand Down
18 changes: 18 additions & 0 deletions test/regression/expected/projection_pushdown_unsupported_type.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
-- XML is not supported, pushdown should avoid problems
CREATE TABLE my_table(a TEXT, b XML, c INTEGER);
INSERT INTO my_table (a, b, c) SELECT * from (
VALUES
('a', '<root><element>value</element></root>'::XML, 42),
(NULL, NULL, NULL),
('b', '<root><element>value</element></root>'::XML, -128),
('c', '<root><element>value</element></root>'::XML, 2000000)
) t(a);
SELECT a, c FROM my_table;
a | c
---+---------
a | 42
|
b | -128
c | 2000000
(4 rows)

1 change: 1 addition & 0 deletions test/regression/schedule
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ test: search_path
test: execution_error
test: type_support
test: array_type_support
test: projection_pushdown_unsupported_type
10 changes: 10 additions & 0 deletions test/regression/sql/projection_pushdown_unsupported_type.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
-- XML is not supported, pushdown should avoid problems
CREATE TABLE my_table(a TEXT, b XML, c INTEGER);
INSERT INTO my_table (a, b, c) SELECT * from (
VALUES
('a', '<root><element>value</element></root>'::XML, 42),
(NULL, NULL, NULL),
('b', '<root><element>value</element></root>'::XML, -128),
('c', '<root><element>value</element></root>'::XML, 2000000)
) t(a);
SELECT a, c FROM my_table;

0 comments on commit b7a1be7

Please sign in to comment.