CrunchyData / pg_parquet

Copy to/from Parquet in S3 from within PostgreSQL
Other
361 stars 11 forks source link

Add pg{14,15} to support matrix #57

Closed aykut-bozkurt closed 1 month ago

aykut-bozkurt commented 1 month ago

3 functions need to be ported to support PG{14,15}.

pg_analyze_and_rewrite for pg14 pg_analyze_and_rewrite_fixedparams for >=pg15

Value for pg14 String for >= pg15

EmitWarningsOnPlaceholders for pg14 MarkGUCPrefixReserved for >= pg15

pgguru commented 1 month ago

CI is now fixed, and Pg 13 is failing for the correct reasons... (Always important to distinguish there... 😁) So doing a little brain dump on what I'm seeing here. I have a theory and an instinct for what is going on, but need to dig more.

So there is a test failure with Pg 13, but it's happening at an unexpected point which is the "validate success" test path:

pg_parquet=# copy factory_correct from '/tmp/test.parquet';
ERROR:  wrong element type
CONTEXT:  COPY factory_correct, line 1, column workers

Per git logs, this sounds like this is related to postgres/postgres@670c0a1d47 which was only backported to 14, so this would be why we're getting this error from PG 13, and the backtrace backs it up:

#0  0x000000000088ebd0 in errfinish ()
#1  0x00000000007a1bfa in array_recv ()
#2  0x0000000000893697 in ReceiveFunctionCall ()
#3  0x00000000005b6480 in NextCopyFrom ()
#4  0x00000000005b6ac3 in CopyFrom ()
#5  0x00007f24564bbe0b in pgrx_pg_sys::include::pg13::CopyFrom::{closure#0} () at target/debug/build/pgrx-pg-sys-4339c96e9dde59a8/out/pg13.rs:31255
#6  0x00007f24566274e3 in pgrx_pg_sys::submodules::ffi::pg_guard_ffi_boundary_impl::{closure#0}<u64, pgrx_pg_sys::include::pg13::CopyFrom::{closure_env#0}> (jump_buffer=0x7ffe700f9ed0) at /home/david/.cargo/registry/src/index.crates.io-6f17d22bba15001f/pgrx-pg-sys-0.12.6/src/submodules/ffi.rs:179
#7  0x00007f2456580af1 in cee_scape::asm_based::call_with_sigsetjmp::call_from_c_to_rust<pgrx_pg_sys::submodules::ffi::pg_guard_ffi_boundary_impl::{closure_env#0}<u64, pgrx_pg_sys::include::pg13::CopyFrom::{closure_env#0}>> (jbuf=0x7ffe700f9ed0, closure_env_ptr=0x7ffe700f9eb8)
    at /home/david/.cargo/registry/src/index.crates.io-6f17d22bba15001f/cee-scape-0.2.0/src/asm_based.rs:186
...
#33 0x000000000077ee16 in exec_simple_query ()
#34 0x0000000000780a99 in PostgresMain ()
#35 0x000000000070a57f in ServerLoop ()
#36 0x000000000070b3e9 in PostmasterMain ()
#37 0x0000000000490b9f in main ()

A possible solution for PG 13 support could be to switch to text mode instead of binary mode here (presumably only for Pg 13), or to forbid arrays of user-defined composite types when using Pg 13. (There may be other solutions, but this seems to be where the issue comes in.)

In any case, I think the recommendation at this point is to merge support for 14-17 for now and open a second PR with 13 support, to the extent we can get it.