Use PostgreSQL nodes for scanning tables #477

mkaruza · 2024-12-04T13:29:51Z

Idea is to reconstruct query based on duckdb filtering information, for each table and use that information to plan postgres execution. This plan will, potentially exeucute with parallel workers. If no workers are available we will scan this local to thread.

This approach has advantage that it also will support all other scan nodes that are available (and postgresql thinks are best - index/index only/bitmap scan, also partitioned tables should be possible)

Fixes #243

JelteF

Initial round of review. I really like the direction of the code. One general feedback is that this definitely needs a bunch of tests too.

src/pgduckdb.cpp

src/pgduckdb_duckdb.cpp

src/pgduckdb_types.cpp

src/scan/postgres_scan.cpp

src/scan/postgres_table_reader.cpp

include/pgduckdb/scan/postgres_table_reader.hpp

src/scan/postgres_table_reader.cpp

include/pgduckdb/pgduckdb_process_latch.hpp

lepsa0321 · 2024-12-14T08:46:36Z

I've been looking into the PR you submitted, and I'm curious about its impact on index-scan functionality. Could you please clarify if this PR addresses the issue where index-scans were not being used when expected?

If it does resolve this problem, could you also provide guidance on how to enforce the use of an index-scan? Specifically:

Are there any configuration settings or parameters that can be adjusted to prefer index-scans over other scan methods?
Is there a way to hint or force the query planner to choose an index-scan for certain queries?
Any additional steps or considerations we should be aware of when trying to ensure that index-scans are utilized?
Your insights would be greatly appreciated as they will help us better understand the improvements and how to leverage them effectively in our environment.

Thank you

mkaruza · 2024-12-14T10:34:21Z

I've been looking into the PR you submitted, and I'm curious about its impact on index-scan functionality. Could you please clarify if this PR addresses the issue where index-scans were not being used when expected?

If it does resolve this problem, could you also provide guidance on how to enforce the use of an index-scan? Specifically:

Are there any configuration settings or parameters that can be adjusted to prefer index-scans over other scan methods? Is there a way to hint or force the query planner to choose an index-scan for certain queries? Any additional steps or considerations we should be aware of when trying to ensure that index-scans are utilized? Your insights would be greatly appreciated as they will help us better understand the improvements and how to leverage them effectively in our environment.

Thank you

Hi, i have added regression test that shows that Bitmap/Index/IndexOnly scan is used when appropriate - hopefully that will help you answer your question.

lepsa0321 · 2024-12-15T13:23:49Z

I've been looking into the PR you submitted, and I'm curious about its impact on index-scan functionality. Could you please clarify if this PR addresses the issue where index-scans were not being used when expected?
If it does resolve this problem, could you also provide guidance on how to enforce the use of an index-scan? Specifically:
Are there any configuration settings or parameters that can be adjusted to prefer index-scans over other scan methods? Is there a way to hint or force the query planner to choose an index-scan for certain queries? Any additional steps or considerations we should be aware of when trying to ensure that index-scans are utilized? Your insights would be greatly appreciated as they will help us better understand the improvements and how to leverage them effectively in our environment.
Thank you

Hi, i have added regression test that shows that Bitmap/Index/IndexOnly scan is used when appropriate - hopefully that will help you answer your question.

Thank you very much~

include/pgduckdb/catalog/pgduckdb_table.hpp

include/pgduckdb/pgduckdb_process_latch.hpp

include/pgduckdb/utility/rename_ruleutils.h

src/scan/postgres_table_reader.cpp

include/pgduckdb/scan/postgres_scan.hpp

src/scan/postgres_scan.cpp

include/pgduckdb/scan/postgres_scan.hpp

src/scan/postgres_table_reader.cpp

# Conflicts: # include/pgduckdb/pgduckdb_guc.h # Conflicts: # src/scan/heap_reader.cpp # src/scan/postgres_seq_scan.cpp

Added WaitLatch that can be used with multiple threads in process.

include/pgduckdb/pgduckdb_guc.h

src/pgduckdb_hooks.cpp

At the very least SlotGetAllAttrs should should be called while holding the lock.

If GlobaclLock is held for duration of populating output vector we don't need any special WaitLatch wrappers and we can call directly this function.

We should now support all types with native postgres scan

* Setting this variable to `0` disables parallelization * Cardinality of table less than 65536 use only single parallel process * Higher cardinality will try to use `max_workers_per_postgres_scan` parallel processes with upper limit of `max_parallel_workers`

mkaruza requested review from Y-- and JelteF December 4, 2024 13:29

mkaruza marked this pull request as draft December 4, 2024 13:29

JelteF reviewed Dec 4, 2024

View reviewed changes

This was referenced Dec 9, 2024

WIP IndexOnlyScan #71

Closed

Make InsertTupleIntoChunk more efficient #416

Closed

Parallel Scan with multiple processes #63

Closed

Move PG code out of filter logic #487

Open

Support for set returning functions that don't exist in DuckDB #483

Open

JelteF reviewed Dec 13, 2024

View reviewed changes

include/pgduckdb/pgduckdb_process_latch.hpp Outdated Show resolved Hide resolved

mkaruza marked this pull request as ready for review December 14, 2024 10:28

mkaruza force-pushed the postgres-native-scan branch from de068c6 to 03b1e3a Compare December 14, 2024 10:33

Y-- reviewed Dec 16, 2024

View reviewed changes

mkaruza added 14 commits December 16, 2024 16:13

Use PostgreSQL execution nodes to scan tables

47535d9

# Conflicts: # include/pgduckdb/pgduckdb_guc.h # Conflicts: # src/scan/heap_reader.cpp # src/scan/postgres_seq_scan.cpp

Simple comment for calculation of wanted number of workers

eab8ab9

Don't allow partitioned table scan for now

82be314

Added GlobalProcessLatch

e86678d

Added WaitLatch that can be used with multiple threads in process.

Renamed DuckdbProcessLock to GlobalProcessLock

93acd1e

Remove PostgresViewScan as it is not used anymore

6e3ef8e

Protect pg function with guard. Handle TEMP tables.

e965bf6

Keep compiler quite for unused variables

72872d3

Minor header cleanup

15e7b05

Regression test for native postgres scan. Handle COUNT(*) in batches.

b71c592

Added small comment

559f0f0

Remove last part of view replacement scan.

0986852

Small output fix

4eac06e

Forgot to include regression test in schedule

48115da

Y-- added 3 commits December 18, 2024 14:30

Fix Mac build

b0d482e

Quote column name

eed0def

Vendor RELKIND_HAS_TABLE_AM for PG14

1302610

This was referenced Dec 18, 2024

Make InsertTupleIntoChunk more efficient #402

Closed

Case insensitivity mismatch between duckdb and postgres can cause issues #116

Open

mkaruza added 3 commits December 19, 2024 11:44

Calculate number of parallel workers on scan for count_tuples_only

d736fde

Fixed regression test for previous commit

3f45be5

Refactor logic for WaitGlobalLatch

ac2e1f2

mkaruza force-pushed the postgres-native-scan branch from e78c6a8 to ac2e1f2 Compare December 20, 2024 09:43

Simplify latch waiting logic

6addf7b

JelteF reviewed Dec 20, 2024

View reviewed changes

include/pgduckdb/pgduckdb_guc.h Outdated Show resolved Hide resolved

JelteF added 2 commits December 20, 2024 14:47

Add comment to PostgresScopedStackReset

47fa442

clang-format off around vendored code

6ddf3e4

Leo-XM-Zeng mentioned this pull request Dec 20, 2024

Add basic test cases about #116 #504

Closed

JelteF added 6 commits December 20, 2024 15:34

Move ConstructFullyQualifiedTableName to relations.cpp

e0d4be3

Remove accidental lock

0e8fa08

Move QuoteIdentifier to relations.cpp

5594d65

Undo accidental removal

1a15616

Merge remote-tracking branch 'origin' into postgres-native-scan

cc47db6

Merge remote-tracking branch 'origin' into postgres-native-scan

4609992

JelteF reviewed Dec 20, 2024

View reviewed changes

src/pgduckdb_hooks.cpp Outdated Show resolved Hide resolved

JelteF and others added 9 commits December 20, 2024 16:01

Make postgres_scan.cpp c++ only

dba08a9

Take GlobalProcessLock for the duration of a chunk

9906d24

At the very least SlotGetAllAttrs should should be called while holding the lock.

GlobalLock held for duration of populating output vector

dfd3dad

If GlobaclLock is held for duration of populating output vector we don't need any special WaitLatch wrappers and we can call directly this function.

Use returned tuple slot attr information for populating output vector

f8bca89

Rename GUC variable according to PR suggestion

fd16493

Remove check for allowed table types

ebface1

We should now support all types with native postgres scan

Fix typo

b72550a

Merge remote-tracking branch 'origin' into postgres-native-scan

ef00ee9

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use PostgreSQL nodes for scanning tables #477

Use PostgreSQL nodes for scanning tables #477

mkaruza commented Dec 4, 2024 •

edited by JelteF

Loading

JelteF left a comment

lepsa0321 commented Dec 14, 2024

mkaruza commented Dec 14, 2024

lepsa0321 commented Dec 15, 2024

Use PostgreSQL nodes for scanning tables #477

Are you sure you want to change the base?

Use PostgreSQL nodes for scanning tables #477

Conversation

mkaruza commented Dec 4, 2024 • edited by JelteF Loading

JelteF left a comment

Choose a reason for hiding this comment

lepsa0321 commented Dec 14, 2024

mkaruza commented Dec 14, 2024

lepsa0321 commented Dec 15, 2024

mkaruza commented Dec 4, 2024 •

edited by JelteF

Loading