Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PREPARE statement doesn't work #51

Closed
2 tasks done
BigBerny opened this issue Jul 18, 2024 · 6 comments · Fixed by #140
Closed
2 tasks done

PREPARE statement doesn't work #51

BigBerny opened this issue Jul 18, 2024 · 6 comments · Fixed by #140
Assignees
Labels
bug Something isn't working good first issue Good for newcomers priority-medium Medium priority issue

Comments

@BigBerny
Copy link

BigBerny commented Jul 18, 2024

What happens?

PREPARE test_query(text) AS
SELECT count(*) FROM predictions where client = $1;
EXECUTE test_query('example');

I get this warning when I run the query and it takes forever:
WARNING: This query was not fully pushed down to DuckDB because DuckDB returned an error. Query times may be impacted. If you would like to see this query pushed down, please submit a request to https://github.com/paradedb/paradedb/issues with the following context:
Not implemented Error: Prepared statement argument types are not supported, use CAST

With this identical query it works in a few milliseconds:

SELECT count(*) FROM predictions where client = 'example';

DuckDB itself supports it: https://duckdb.org/docs/api/c/prepared.html

To Reproduce

Run the query from above

OS:

Ubuntu

ParadeDB Version:

v0.8.4

Are you using ParadeDB Docker, Helm, or the extension(s) standalone?

ParadeDB pg_lakehouse Extension

Full Name:

Janis

Affiliation:

Typewise

Did you include all relevant data sets for reproducing the issue?

Yes

Did you include the code required to reproduce the issue?

  • Yes, I have

Did you include all relevant configurations (e.g., CPU architecture, PostgreSQL version, Linux distribution) to reproduce the issue?

  • Yes, I have
@BigBerny BigBerny added the bug Something isn't working label Jul 18, 2024
@philippemnoel philippemnoel added the good first issue Good for newcomers label Jul 19, 2024
@philippemnoel
Copy link
Collaborator

Thank you for reporting, we'll get this fixed ASAP.

@kysshsy
Copy link
Contributor

kysshsy commented Jul 20, 2024

AFAIK paradedb don't support pushdown PREPARE statement to duckdb now. In the executor hook, it should follow the prev_hook branch and then goes the FDW (this could avoid the warning).

When executing "Execute test_query('example');", the query_desc.operation is CmdType_CMD_SELECT, but query text is "prepare xxx AS query ". So the code fail.

I think there are two ways to solve this problem

  1. goes FDW(this avoid warning)
  2. construct the query string and then send to duckdb

@kysshsy
Copy link
Contributor

kysshsy commented Aug 8, 2024

Currently, the utility infrastructure is facing issues that need to be addressed:

  1. Check the meta data of foreign tables
    All relation metadata in DuckDB is also stored in PostgreSQL. We need to verify that these relations exist and ensure the query is safe for pushdown to DuckDB.
  2. Make sure relations have been loaded
    DuckDB lazily loads views (relations), so it's crucial to confirm that these relations have been created before pushing DDL operations (such as PREPARE and EXPLAIN) to DuckDB.
  3. Set search path correctly
    If tables in the query are not schema-qualified, we need to set the search path correctly to ensure DuckDB can locate them.

Actually the issues mentioned above not only apply to the utility_hook but also in the executor_hook. However, they have been addressed by FDW and PostgreSQL query analysis. Now, we need to address them specifically within the utility hook.

@philippemnoel philippemnoel transferred this issue from paradedb/paradedb Aug 8, 2024
@philippemnoel philippemnoel added the priority-medium Medium priority issue label Aug 8, 2024
@philippemnoel
Copy link
Collaborator

Currently, the utility infrastructure is facing issues that need to be addressed:

  1. Check the meta data of foreign tables
    All relation metadata in DuckDB is also stored in PostgreSQL. We need to verify that these relations exist and ensure the query is safe for pushdown to DuckDB.
  2. Make sure relations have been loaded
    DuckDB lazily loads views (relations), so it's crucial to confirm that these relations have been created before pushing DDL operations (such as PREPARE and EXPLAIN) to DuckDB.
  3. Set search path correctly
    If tables in the query are not schema-qualified, we need to set the search path correctly to ensure DuckDB can locate them.

Actually the issues mentioned above not only apply to the utility_hook but also in the executor_hook. However, they have been addressed by FDW and PostgreSQL query analysis. Now, we need to address them specifically within the utility hook.

Thank you for highlighting this. Let's handle them first as you suggest. I'm excited for us to fix these limitations in the utility hook.

@philippemnoel
Copy link
Collaborator

Currently, the utility infrastructure is facing issues that need to be addressed:

  1. Check the meta data of foreign tables
    All relation metadata in DuckDB is also stored in PostgreSQL. We need to verify that these relations exist and ensure the query is safe for pushdown to DuckDB.
  2. Make sure relations have been loaded
    DuckDB lazily loads views (relations), so it's crucial to confirm that these relations have been created before pushing DDL operations (such as PREPARE and EXPLAIN) to DuckDB.
  3. Set search path correctly
    If tables in the query are not schema-qualified, we need to set the search path correctly to ensure DuckDB can locate them.

Actually the issues mentioned above not only apply to the utility_hook but also in the executor_hook. However, they have been addressed by FDW and PostgreSQL query analysis. Now, we need to address them specifically within the utility hook.

Is this unblocked now that the EXPLAIN PR was merged?

@kysshsy
Copy link
Contributor

kysshsy commented Sep 23, 2024

Yes, I'm working on pgrx to add some functions related to Prepare. You can check it out in my pull request here: pgcentralfoundation/pgrx#1873. After that, I’ll be able to resume development.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers priority-medium Medium priority issue
Projects
None yet
3 participants