MisterRaindrop opened a new pull request, #1951:
URL: https://github.com/apache/cloudberry/pull/1951
### What does this PR do?
`contrib/datalake_fdw` landed in #1842 with a format layer that had an
interface and no implementation. This is the Parquet one, plus the two
conversions either side of it: PostgreSQL tuples into an Arrow batch, and an
Arrow batch back into Datums.
Parquet is reached through Arrow rather than libparquet on its own, because
libparquet is written in terms of Arrow's types -- linking one links the
other
-- and going around Arrow would mean re-deriving the definition and
repetition
levels, the timestamp unit rules and the several ways a decimal can be stored
that `arrow::parquet` already gets right.
Design points worth a reviewer's attention:
- **A fragment is a range of row groups, not a whole file**, so that one
large
file can be read by several segments at once. File-at-a-time assignment
cannot express that, and it is the granularity the parallel scan will need.
- **Reading is single-threaded on purpose.** Arrow will read column chunks in
parallel if asked, and a worker thread that hits an error has no way to
report
it through PostgreSQL's error handling.
- **A row group is held until it is complete**, because Parquet cannot begin
one
it does not have. `iceberg.batch_rows` (default 16384) is how many rows
cross
the boundary at a time.
- **The Arrow C data interface is the C/C++ boundary.** The writer side is
C++
and never calls a PostgreSQL allocator; the reader side is C and reads the
Arrow buffers directly, so an allocation failure unwinds through C frames
only.
- **A reader and a writer hold a file descriptor and memory from Arrow's
allocator**, neither of which transaction abort reclaims, so both register
with the resource owner -- the shape PAX uses in `comm/pax_resource.cc`,
because this server does not carry PostgreSQL 16's typed resource-kind API.
- **Dates and timestamps shift by 30 years** between PostgreSQL's epoch and
Arrow's, and the shift is range-checked in *both* directions before it is
applied. PostgreSQL's range runs about 34 years past what Arrow can hold as
microseconds from 1970, and a round trip through two unchecked halves would
agree with itself.
Types supported: `bool`, the four integers, both floats, `text`, `varchar`,
`char`, `bytea`, `date`, `timestamp`, `timestamptz`. Anything else is
refused by
name rather than approximated.
### Type of Change
- [ ] Bug fix (non-breaking change)
- [x] New feature (non-breaking change)
- [ ] Breaking change (fix or feature with breaking changes)
- [ ] Documentation update
### Test Plan
`datalake_fdw_test` is a second extension in the same shared library, holding
the two functions this can be exercised with from SQL -- write the result of
a
query to a file, read a file back as rows. It is not part of what
`datalake_fdw` installs. Until the access method is finished there is no
other
way to run the format layer in a real backend.
- [x] Integration tests added/updated -- a new `format_parquet` regression
category that round-trips every supported type through a local Parquet
file, including nulls and values on both sides of 1970, and checks that
reading row groups 0, 1 and 2 separately gives back exactly what
reading
the file whole does. Refusals are covered too: an unsupported column
type,
the wrong column type on read, a column count that does not match, a
row
group range past the end of the file, a timestamp Arrow cannot hold,
and a
negative or oversized row group size.
- [x] Passed `make installcheck` -- 4/4 (`parquet_roundtrip` plus the three
existing `iceberg_am` cases), against a three-segment cluster with the
module in `shared_preload_libraries`.
- [ ] Unit tests added/updated
- [ ] Passed `make -C src/test installcheck-cbdb-parallel` (not run)
Two things checked outside the regression suite:
- **Interoperability.** Files written here are read back correctly by
**pyarrow 21** -- a different Arrow implementation than the one that wrote
them (9.0.0), which is what makes the round trip mean something rather than
agreeing with itself. Every type comes back with the right value, including
`char(5)` padding and microsecond timestamps on both sides of 1970. The
Arrow
schema is deliberately *not* stored in the file's metadata, so what comes
back
is what any other reader of the file sees rather than what we noted down.
- **Arrow version spread.** Arrow 9.0.0 from EPEL 9 builds and passes the
tests.
The Arrow-facing translation units also compile without warnings against
17.0.0 on Rocky 10 with gcc 14, and against 17.0.0 and 21.0.0 on Rocky 8
with
gcc 8. Those two are compile-only checks, not test runs.
### Impact
**Dependencies:** new build dependency on the **Arrow and Parquet C++
libraries**, found with pkg-config. `contrib/datalake_fdw` is off by default
(`--enable-datalake-fdw`) and is not compiled into the RPM, so this does not
change what the packages need; the CI job that builds the extension with PGXS
installs them. On Rocky 9 and 10 they come from EPEL. On Rocky 8 they come
from
the Arrow project's own repository, pinned to 17.0.0, because EPEL 8's
`libarrow-devel` needs a `utf8proc-devel` that modular filtering keeps out of
PowerTools, and because the newest Arrow wants C++20, which gcc 8 does not
have.
One flag worth naming: whatever `-std=` Arrow's `.pc` file asks for is
filtered
out. pkg-config's cflags land in `CPPFLAGS`, which `pgxs.mk` puts *after*
`CXXFLAGS`, so the Arrow project's own `-std=c++11` would otherwise win over
the
`-std=c++17` this module is written for -- and Arrow's headers then fail to
compile against themselves, in a way that reads like the library needing a
newer
compiler.
**User-facing changes:** one new setting, `iceberg.batch_rows`. Nothing else
is
reachable yet: the access method still reports "not supported" for anything
that
would touch data, so the format layer has no user-facing path into it.
**Performance:** not measured. Nothing calls this layer on a query path yet.
### Checklist
- [x] Followed [contribution
guide](https://cloudberry.apache.org/contribute/code)
- [ ] Added/updated documentation
- [x] Reviewed code for security implications
- [x] This PR contains AI-assisted code generation
- [ ] Requested review from [cloudberry
committers](https://github.com/orgs/apache/teams/cloudberry-committers)
On security: `datalake_parquet_write(path, query)` names a path on the
server's
file system and runs a query through SPI, so it is as privileged as
`pg_read_server_files` and is granted the same way -- `REVOKE EXECUTE ...
FROM
PUBLIC`, superuser only. It ships in `datalake_fdw_test`, which is a separate
extension precisely so that installing `datalake_fdw` does not put it in a
database; the honest limit is that `make install` still puts its control
file in
the share directory, since PGXS's `NO_INSTALL` is per-module and the
functions
have to live in the library whose internals they test.
### Additional Context
Deliberately left for later, so a reviewer knows they are choices and not
oversights:
- **Local files only.** `arrow::io::ReadableFile` directly. Object storage
arrives as an `arrow::io::RandomAccessFile` over the storage facade in
`common/file_system_wrapper.h`, and `parquet_read.cpp` and
`parquet_write.cpp` are the only files that have to change when it does.
- **No row group pruning.** `open_reader` *refuses* a non-NULL filter set
rather
than ignoring it: ignoring it would still give the right rows, so nothing
would fail, which is exactly why a caller that believed the pruning had
happened would have no way to find out.
- **No NUMERIC.** DECIMAL has four storage forms in Parquet and deserves its
own
change; it is refused by column name today.
- **The merge-on-read row ordinal** that positional deletes match against is
not
produced yet; it arrives with them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
