paradedb / pg_analytics

DuckDB-powered analytics for Postgres
https://paradedb.com
PostgreSQL License
383 stars 15 forks source link

feat: Support for views #132

Closed Weijun-H closed 3 weeks ago

Weijun-H commented 1 month ago

Ticket(s) Closed

What

CREATE VIEW AS SELECT ... FROM FOREIGN_TABLE do not push down to DuckDB

Why

How

In the process_utility_hook, push this view to DuckDB

Tests

test_view_foreign_table in tests/scan.rs

Weijun-H commented 1 month ago
---- test_view_foreign_table stdout ----
thread 'test_view_foreign_table' panicked at tests/scan.rs:615:9:
assertion `left == right` failed
  left: "error returned from database: column trips.vendorid does not exist"
 right: "WARNING: public.rate_code does not exist in DuckDB"

~cannot reproduce the CI error on my M3 locally~

philippemnoel commented 1 month ago
---- test_view_foreign_table stdout ----
thread 'test_view_foreign_table' panicked at tests/scan.rs:615:9:
assertion `left == right` failed
  left: "error returned from database: column trips.vendorid does not exist"
 right: "WARNING: public.rate_code does not exist in DuckDB"

cannot reproduce the CI error on my M3 locally

That's strange, I would expect this kind of error to not be environment-specific.

Weijun-H commented 1 month ago

Thanks for your feedback @kysshsy , I will rethink this pr

philippemnoel commented 1 month ago

@Weijun-H Is this ready to review again?

Weijun-H commented 1 month ago

@Weijun-H Is this ready to review again?

Yes, it is ready for review.

philippemnoel commented 1 month ago

@Weijun-H Is this ready to review again?

Yes, it is ready for review.

@kysshsy Would you mind doing a second review? Thank you 🙏

Weijun-H commented 3 weeks ago

~Tests passed locally, I'm not sure why they failed in CI.~ Fixed

Weijun-H commented 3 weeks ago

I used sqlparser to parse the query and iterate through the relation table for better control. All tests have passed, including those for nested views and joins between foreign and regular tables.

Ready for review again.

kysshsy commented 3 weeks ago

The error when using the pg function for rewrite was due to pg updating the Utility structure in place. Here, we can perform a deep copy to preserve the original Utility structure.

 let utility_stmt = unsafe {pg_sys::copyObjectImpl(pstmt.utilityStmt as *const c_void) as *mut pg_sys::Node};
            prev_hook(
                pstmt.clone(),
                query_string,
                read_only_tree,
                context,
                params,
                query_env,
                dest,
                completion_tag,
            );
            view_query(
            query_string,
            utility_stmt as *mut pg_sys::ViewStmt,
            pstmt.stmt_location,
            pstmt.stmt_len,
        )?
kysshsy commented 3 weeks ago

LGTM!