diff --git a/.github/workflows/build-cloudberry.yml b/.github/workflows/build-cloudberry.yml index 83dcad822ad..e0a23385b53 100644 --- a/.github/workflows/build-cloudberry.yml +++ b/.github/workflows/build-cloudberry.yml @@ -1440,6 +1440,63 @@ jobs: echo "Exact KB:" df -k / + # contrib/datalake_fdw reads and writes s3 through the AWS SDK for C++, + # which no distribution packages, so it is built from source. That costs + # a few minutes, and the result depends on nothing but the version, the + # distribution and the architecture -- so it is cached under exactly + # those three, and a hit restores it in seconds. + - name: Restore the AWS SDK for C++ + id: aws-sdk-cache + if: success() && needs.check-skip.outputs.should_skip != 'true' && matrix.pgxs_extension == 'contrib/datalake_fdw' + uses: actions/cache@v4 + with: + path: /opt/datalake + key: aws-sdk-cpp-1.11.844-rocky${{ matrix.rocky_version }}-${{ runner.arch }} + + - name: Build the AWS SDK for C++ + if: success() && needs.check-skip.outputs.should_skip != 'true' && matrix.pgxs_extension == 'contrib/datalake_fdw' && steps.aws-sdk-cache.outputs.cache-hit != 'true' + run: | + set -eo pipefail + + # Only s3 and the sts it needs to assume a role: the whole SDK is + # some three hundred clients and none of the rest is reachable from + # here. Static and position-independent, because what consumes it is + # a shared library that promises to add no runtime dependency of its + # own beyond the system libraries the SDK itself needs. + . /etc/os-release + crb=crb + if [[ "${VERSION_ID%%.*}" == "8" ]]; then crb=powertools; fi + + # Only what is missing, asked for by capability so a package the + # distribution renamed still counts as present -- Rocky 10 ships + # zlib-devel as zlib-ng-compat-devel. Naming a package the image + # already has makes dnf try to upgrade it to the newest build in the + # repository, and that is how this step first failed: Rocky 10's + # newest libcurl-devel wants a libcurl no enabled repository + # carries. The intent is to add build dependencies, never to move + # the image's own packages. + missing="" + for pkg in cmake git ninja-build libcurl-devel openssl-devel zlib-devel; do + rpm -q --whatprovides "${pkg}" > /dev/null 2>&1 || missing="${missing} ${pkg}" + done + if [[ -n "${missing}" ]]; then + dnf install -y --enablerepo=epel --enablerepo=${crb} ${missing} + fi + + git clone --depth 1 --branch 1.11.844 \ + --recurse-submodules --shallow-submodules \ + https://github.com/aws/aws-sdk-cpp.git /tmp/aws-sdk-cpp + cmake -S /tmp/aws-sdk-cpp -B /tmp/aws-sdk-cpp/build -GNinja \ + -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=/opt/datalake \ + -DBUILD_ONLY="s3;sts" -DBUILD_SHARED_LIBS=OFF \ + -DCMAKE_POSITION_INDEPENDENT_CODE=ON \ + -DENABLE_TESTING=OFF -DUSE_OPENSSL=ON + ninja -C /tmp/aws-sdk-cpp/build install + + # The cache stores what is under /opt/datalake; the tree it was built + # from is a gigabyte that nothing reads again. + rm -rf /tmp/aws-sdk-cpp + # Modules that ship as extensions are not built into the RPM. Build them # here the way a user would: against the server that was just installed, # through PGXS, with no configured source tree involved. This has to run @@ -1488,11 +1545,21 @@ jobs: dnf install -y --enablerepo=epel --enablerepo=powertools \ arrow-devel-17.0.0-1.el8 parquet-devel-17.0.0-1.el8 else - # From EPEL, which the image has enrolled but left disabled, - # exactly as it does for its own EPEL packages; CRB carries what - # they depend on. + # Apache's own repository rather than EPEL, and pinned. EPEL's + # Arrow moves when EPEL moves, so a version change there would + # arrive in this job with no commit of ours behind it -- and + # Arrow is the library this extension's ABI is shared with. The + # two versions differ on purpose: between them and the Arrow 9 + # the maintainers test locally, the three legs cover the range + # contrib/datalake_fdw claims to build against. + arrow_version=17.0.0-1.el9 + if [[ "${VERSION_ID%%.*}" == "10" ]]; then + arrow_version=21.0.0-1.el10 + fi + dnf install -y \ + https://apache.jfrog.io/artifactory/arrow/almalinux/${VERSION_ID%%.*}/apache-arrow-release-latest.rpm dnf install -y --enablerepo=epel --enablerepo=crb \ - libarrow-devel parquet-libs-devel + "arrow-devel-${arrow_version}" "parquet-devel-${arrow_version}" fi fi @@ -1540,6 +1607,96 @@ jobs: } 2>&1 | tee -a build-logs/details/create-cloudberry-demo-cluster.log + # The s3 half of contrib/datalake_fdw's regression needs something that + # speaks S3. SeaweedFS is one static binary, starts in a second, and + # answers the same requests as the object stores the maintainers test + # against by hand. Its coordinates go into the job environment, which is + # what the test step reads to decide whether the s3 cases run at all -- + # so every other test entry is unaffected, and a leg where this step did + # not run skips them rather than failing. + - name: Start an S3 service for datalake_fdw + if: success() && needs.check-skip.outputs.should_skip != 'true' && matrix.test == 'ic-datalake-fdw' + run: | + set -eo pipefail + + case "$(uname -m)" in + x86_64) weed_arch=linux_amd64 ;; + aarch64) weed_arch=linux_arm64 ;; + *) echo "::error::no SeaweedFS build for $(uname -m)"; exit 1 ;; + esac + + # Pinned, and to the version the maintainers ran the same suite + # against by hand: a service the tests assert against must not change + # underneath them. + curl -fsSL -o /tmp/seaweedfs.tar.gz \ + "https://github.com/seaweedfs/seaweedfs/releases/download/4.47/${weed_arch}.tar.gz" + tar -C /usr/local/bin -xzf /tmp/seaweedfs.tar.gz weed + rm -f /tmp/seaweedfs.tar.gz + + # One identity with a key pair, because the suite asserts that a + # wrong secret is refused. Started without this file SeaweedFS + # accepts anything, and that case would pass by not being tested. + mkdir -p /tmp/seaweedfs + printf '%s\n' '{"identities":[{"name":"datalake","credentials":[{"accessKey":"datalake","secretKey":"datalake-secret"}],"actions":["Admin","Read","Write","List","Tagging"]}]}' \ + > /tmp/seaweedfs/s3.json + + nohup weed server -dir=/tmp/seaweedfs -ip=127.0.0.1 \ + -master.port=9333 -volume.port=8080 -filer -filer.port=8888 \ + -s3 -s3.port=8333 -s3.config=/tmp/seaweedfs/s3.json \ + > /tmp/seaweedfs/weed.log 2>&1 & + disown + + # Up to a minute, checking rather than sleeping, and checking the S3 + # port itself: the master elects itself and the volume server + # registers seconds before the S3 gateway starts listening, so a + # master that answers is not yet a service the tests can use. An + # unsigned request is refused with 403 once the identity file is in + # effect -- which is the state the wrong-secret case needs -- so any + # HTTP status means listening, and only "000" means nothing answered. + for _ in $(seq 60); do + s3_status=$(curl -sS -o /dev/null -w '%{http_code}' \ + http://127.0.0.1:8333/ 2>/dev/null || true) + if [[ -n "${s3_status}" && "${s3_status}" != "000" ]]; then + break + fi + sleep 1 + done + if [[ -z "${s3_status:-}" || "${s3_status}" == "000" ]]; then + echo "::error::the SeaweedFS S3 gateway never started listening" + tail -50 /tmp/seaweedfs/weed.log + exit 1 + fi + echo "S3 gateway answers an unsigned GET with ${s3_status}" + + # "weed shell" exits 0 whatever happened, so what is checked is the + # bucket being listed afterwards, not an exit code. + echo 's3.bucket.create -name datalake-test' | + weed shell -master=127.0.0.1:9333 > /tmp/seaweedfs/shell.log 2>&1 + if ! echo 's3.bucket.list' | + weed shell -master=127.0.0.1:9333 2>/dev/null | + grep -q 'datalake-test'; then + echo "::error::SeaweedFS has no datalake-test bucket" + cat /tmp/seaweedfs/shell.log + tail -50 /tmp/seaweedfs/weed.log + exit 1 + fi + + { + echo "DATALAKE_TEST_S3_ENDPOINT=http://127.0.0.1:8333" + echo "DATALAKE_TEST_S3_BUCKET=datalake-test" + echo "DATALAKE_TEST_S3_ACCESS_KEY=datalake" + echo "DATALAKE_TEST_S3_SECRET_KEY=datalake-secret" + echo "DATALAKE_TEST_S3_REGION=us-east-1" + echo "DATALAKE_TEST_S3_PATH_STYLE=true" + # A value that is a secret only to this run: the suite asserts it + # never appears in an error message, and the logs are searched for + # it afterwards. + echo "DATALAKE_TEST_S3_BAD_SECRET=dl-canary-${GITHUB_RUN_ID}-${GITHUB_RUN_ATTEMPT}" + # Eleven hundred objects through a paginated listing: minutes, so + # it runs here rather than in every local build. + echo "DATALAKE_TEST_S3_PAGINATION=1" + } >> "${GITHUB_ENV}" + - name: "Run Tests: ${{ matrix.test }}" if: success() && needs.check-skip.outputs.should_skip != 'true' env: @@ -1634,6 +1791,21 @@ jobs: PG_OPTS="$PG_OPTS -c default_table_access_method=${{ matrix.pg_settings.default_table_access_method }}" fi + # A test that needs a service of its own carries its coordinates in + # the job environment. "su -" builds a login shell and drops all of + # it, so what the tests need has to be named on the command line + # below; every other entry leaves this empty and the command line is + # the one it always was. + EXTRA_TEST_ENV="" + if [[ -n "${DATALAKE_TEST_S3_ENDPOINT:-}" ]]; then + for v in DATALAKE_TEST_S3_ENDPOINT DATALAKE_TEST_S3_BUCKET \ + DATALAKE_TEST_S3_ACCESS_KEY DATALAKE_TEST_S3_SECRET_KEY \ + DATALAKE_TEST_S3_REGION DATALAKE_TEST_S3_PATH_STYLE \ + DATALAKE_TEST_S3_BAD_SECRET DATALAKE_TEST_S3_PAGINATION; do + EXTRA_TEST_ENV+="${v}='${!v}' " + done + fi + # Read configs into array IFS=' ' read -r -a configs <<< "${{ join(matrix.make_configs, ' ') }}" @@ -1660,6 +1832,7 @@ jobs: # Execute test script with proper environment setup if ! time su - gpadmin -c "cd ${SRC_DIR} && \ + ${EXTRA_TEST_ENV}\ MAKE_NAME='${{ matrix.test }}-config$i' \ MAKE_TARGET='$target' \ MAKE_DIRECTORY='-C $dir' \ diff --git a/contrib/datalake_fdw/Makefile b/contrib/datalake_fdw/Makefile index a9ad29cd33e..567e4b1a602 100644 --- a/contrib/datalake_fdw/Makefile +++ b/contrib/datalake_fdw/Makefile @@ -51,10 +51,19 @@ OBJS = \ src/common/dl_option_util.o \ src/common/parser_option.o \ src/common/file_system_wrapper.o \ + src/common/local_file_system.o \ src/common/s3_file_system.o \ src/common/backend_registry.o \ + src/test/storage_test_backend.o \ src/test/datalake_fdw_test.o +HEADERS = \ + src/common/storage_backend.h \ + src/common/storage_backend_register.h \ + src/common/datalake_location.h \ + src/common/dl_kv.h \ + src/common/dl_err.h + # libparquet is written in terms of Arrow's types, so linking one links both. PKG_CONFIG ?= pkg-config ARROW_MODULES = arrow parquet @@ -81,13 +90,57 @@ Debian and Ubuntu) endif endif +# The AWS SDK for C++, which the s3 backend is built on. No distribution +# packages it, so it is found by prefix rather than by pkg-config: either the +# one named on the command line, or the usual places a hand-built one lands. +# Without it the module still builds, and opening an s3:// location is what +# reports that it was left out. +AWS_SDK_SEARCH = /usr/local /opt/datalake +ifeq ($(origin AWS_SDK_PREFIX),undefined) +AWS_SDK_PREFIX := $(firstword $(foreach d,$(AWS_SDK_SEARCH),\ + $(if $(wildcard $(d)/include/aws/s3/S3Client.h),$(d)))) +else +# Named explicitly and wrong is a mistake to report, not to work around. +ifeq ($(wildcard $(AWS_SDK_PREFIX)/include/aws/s3/S3Client.h),) +ifeq ($(filter clean distclean maintainer-clean,$(MAKECMDGOALS)),) +$(error AWS_SDK_PREFIX=$(AWS_SDK_PREFIX) has no include/aws/s3/S3Client.h) +endif +endif +endif + +ifneq ($(AWS_SDK_PREFIX),) +AWS_SDK_CPPFLAGS = -DDL_HAVE_AWS_SDK -I$(AWS_SDK_PREFIX)/include +# One group, because these static archives refer to each other both ways. +# -Bstatic, because a prefix that also holds shared SDK libraries would +# otherwise bind those, and this module promises to add no runtime dependency +# beyond the system libraries the SDK itself needs. +AWS_SDK_LIBS = -L$(AWS_SDK_PREFIX)/lib64 -L$(AWS_SDK_PREFIX)/lib \ + -Wl,-Bstatic \ + -Wl,--start-group \ + -laws-cpp-sdk-s3 -laws-cpp-sdk-core \ + -laws-crt-cpp -laws-c-s3 -laws-c-auth -laws-c-http -laws-c-io \ + -laws-c-cal -laws-c-compression -laws-c-mqtt -laws-c-event-stream \ + -laws-c-sdkutils -laws-c-common -laws-checksums -ls2n \ + -Wl,--end-group \ + -Wl,-Bdynamic \ + -lssl -lcrypto -lz -lcurl +else +AWS_SDK_CPPFLAGS = +AWS_SDK_LIBS = +ifeq ($(filter clean distclean maintainer-clean,$(MAKECMDGOALS)),) +$(info NOTICE: the AWS SDK for C++ was not found in $(AWS_SDK_SEARCH), so \ +datalake_fdw is being built without the s3 storage backend. Point \ +AWS_SDK_PREFIX at an installed SDK to build with it.) +endif +endif + # Use the documented PGXS knobs: pgxs.mk appends these AFTER the flags configure # chose, so optimization/warning settings survive. A pre-include # "override CFLAGS +=" would give CFLAGS override origin and silently discard # Makefile.global's own "CFLAGS = @CFLAGS@" assignment. PG_CFLAGS = -fvisibility=hidden PG_CXXFLAGS = -fvisibility=hidden -fvisibility-inlines-hidden -std=c++17 -PG_CPPFLAGS = -I$(srcdir)/src $(ARROW_CPPFLAGS) +PG_CPPFLAGS = -I$(srcdir)/src $(ARROW_CPPFLAGS) $(AWS_SDK_CPPFLAGS) # The regression cases live with the rest of the test material rather than in a # second place of their own; pg_regress is pointed at them. REGRESS_OPTS is @@ -106,6 +159,20 @@ REGRESS_OPTS = --temp-config=$(srcdir)/datalake_fdw.conf \ # A second category, and pg_regress takes one --inputdir, so it is a second run. FORMAT_PARQUET_REGRESS = parquet_roundtrip FORMAT_PARQUET_INPUTDIR = $(srcdir)/test/automation/sqlrepo/smoke/format_parquet +STORAGE_LOCAL_REGRESS = storage_local +STORAGE_LOCAL_INPUTDIR = $(srcdir)/test/automation/sqlrepo/smoke/storage_local +# The s3 category needs a service to talk to, so it runs where one is named. +STORAGE_S3_REGRESS = storage_s3 +# Thousands of requests, so it runs where someone asked for it rather than on +# every build; CI does, a local gate does not. +ifneq ($(DATALAKE_TEST_S3_PAGINATION),) +STORAGE_S3_REGRESS += storage_s3_pagination +endif +STORAGE_S3_INPUTDIR = $(srcdir)/test/automation/sqlrepo/smoke/storage_s3 +# One body of storage behaviour, run against each backend that can be reached. +STORAGE_CONFORMANCE_REGRESS = conformance_file conformance_dltest volume_resolve +STORAGE_CONFORMANCE_S3_REGRESS = conformance_s3 +STORAGE_CONFORMANCE_INPUTDIR = $(srcdir)/test/automation/sqlrepo/smoke/storage_conformance EXTRA_CLEAN = exports_darwin.list exports.map @@ -129,7 +196,7 @@ endif # Shared libraries are linked with $(CC) (see src/Makefile.shlib COMPILER), so a # module containing C++ translation units must pull in the C++ runtime itself. -SHLIB_LINK += -lstdc++ $(ARROW_LIBS) +SHLIB_LINK += -lstdc++ $(ARROW_LIBS) $(AWS_SDK_LIBS) # The export list is the single place that decides what stays visible -- which # now also means none of Arrow's symbols become symbols this module offers. @@ -157,7 +224,12 @@ $(shlib): $(EXPORT_LIST) # wins over the one in REGRESS_OPTS. submake and REGRESS_PREP are the same # prerequisites pgxs.mk gives its own targets, so that a parallel make cannot # start pg_regress before it has been built. -installcheck: installcheck-format-parquet +# Each category is a separate pg_regress run against the same database, so +# they have to go one at a time: "make -j" would otherwise start both, and +# two runs creating the same extension in the same database race. The +# categories are chained through their prerequisites rather than listed side +# by side, which is what keeps the order under a parallel make. +installcheck: installcheck-storage-conformance installcheck-format-parquet: submake $(REGRESS_PREP) $(pg_regress_installcheck) $(REGRESS_OPTS) \ @@ -165,14 +237,68 @@ installcheck-format-parquet: submake $(REGRESS_PREP) .PHONY: installcheck-format-parquet +installcheck-storage-local: submake $(REGRESS_PREP) installcheck-format-parquet + $(pg_regress_installcheck) $(REGRESS_OPTS) \ + --inputdir=$(STORAGE_LOCAL_INPUTDIR) $(STORAGE_LOCAL_REGRESS) + +.PHONY: installcheck-storage-local + +installcheck-storage-s3: submake $(REGRESS_PREP) installcheck-storage-local +ifeq ($(DATALAKE_TEST_S3_ENDPOINT),) + @echo "NOTICE: DATALAKE_TEST_S3_ENDPOINT is not set, skipping the s3 storage tests" +else + $(pg_regress_installcheck) $(REGRESS_OPTS) \ + --inputdir=$(STORAGE_S3_INPUTDIR) $(STORAGE_S3_REGRESS) +endif + +.PHONY: installcheck-storage-s3 + +installcheck-storage-conformance: submake $(REGRESS_PREP) installcheck-storage-s3 + $(pg_regress_installcheck) $(REGRESS_OPTS) \ + --inputdir=$(STORAGE_CONFORMANCE_INPUTDIR) $(STORAGE_CONFORMANCE_REGRESS) +ifneq ($(DATALAKE_TEST_S3_ENDPOINT),) + $(pg_regress_installcheck) $(REGRESS_OPTS) \ + --inputdir=$(STORAGE_CONFORMANCE_INPUTDIR) $(STORAGE_CONFORMANCE_S3_REGRESS) +endif + +.PHONY: installcheck-storage-conformance + # "make check" is in-tree only -- under PGXS pgxs.mk refuses the target -- and # it is the only run that supplies the temp-config that preloads this module. ifndef USE_PGXS -check: check-format-parquet +# Chained for the same reason as installcheck above, and here it matters more: +# every category would otherwise start its own temp instance in ./tmp_check. +check: check-storage-conformance check-format-parquet: submake $(REGRESS_PREP) $(pg_regress_check) $(REGRESS_OPTS) \ --inputdir=$(FORMAT_PARQUET_INPUTDIR) $(FORMAT_PARQUET_REGRESS) .PHONY: check-format-parquet + +check-storage-local: submake $(REGRESS_PREP) check-format-parquet + $(pg_regress_check) $(REGRESS_OPTS) \ + --inputdir=$(STORAGE_LOCAL_INPUTDIR) $(STORAGE_LOCAL_REGRESS) + +.PHONY: check-storage-local + +check-storage-s3: submake $(REGRESS_PREP) check-storage-local +ifeq ($(DATALAKE_TEST_S3_ENDPOINT),) + @echo "NOTICE: DATALAKE_TEST_S3_ENDPOINT is not set, skipping the s3 storage tests" +else + $(pg_regress_check) $(REGRESS_OPTS) \ + --inputdir=$(STORAGE_S3_INPUTDIR) $(STORAGE_S3_REGRESS) +endif + +.PHONY: check-storage-s3 + +check-storage-conformance: submake $(REGRESS_PREP) check-storage-s3 + $(pg_regress_check) $(REGRESS_OPTS) \ + --inputdir=$(STORAGE_CONFORMANCE_INPUTDIR) $(STORAGE_CONFORMANCE_REGRESS) +ifneq ($(DATALAKE_TEST_S3_ENDPOINT),) + $(pg_regress_check) $(REGRESS_OPTS) \ + --inputdir=$(STORAGE_CONFORMANCE_INPUTDIR) $(STORAGE_CONFORMANCE_S3_REGRESS) +endif + +.PHONY: check-storage-conformance endif diff --git a/contrib/datalake_fdw/README.md b/contrib/datalake_fdw/README.md new file mode 100644 index 00000000000..b9a7bec71f4 --- /dev/null +++ b/contrib/datalake_fdw/README.md @@ -0,0 +1,217 @@ + + +# datalake_fdw + +Apache Iceberg lake tables as a Cloudberry extension. This document covers +the storage layer: where a lake table's files live, how the extension is told +to reach them, and how to add a kind of storage it does not know about. + +## Volumes + +A volume is a foreign server that says where data lives and how to get in. +Every path the extension reads or writes belongs to one. + +```sql +CREATE SERVER warehouse + FOREIGN DATA WRAPPER iceberg_volume_fdw + OPTIONS (base_path 's3://analytics/warehouse', + endpoint 'https://s3.eu-central-1.amazonaws.com', + region 'eu-central-1', + path_style_access 'false'); + +CREATE USER MAPPING FOR analyst SERVER warehouse + OPTIONS (access_key_id 'AKIA...', secret_access_key '...'); +``` + +Server options: + +| Option | Meaning | +|---|---| +| `base_path` | Required. A URI: `s3://bucket/prefix` or `file:///mnt/warehouse`. Its scheme selects the storage backend, so there is no separate option naming the protocol. | +| `endpoint` | The service to talk to, when it is not AWS. `http://` selects plain HTTP; anything else is HTTPS. | +| `region` | Defaults to `us-east-1`. | +| `path_style_access` | `true` addresses a bucket as a path, `false` as a host name. Unset means path style when an `endpoint` is given and host style when it is not, which is what AWS and everything else respectively want. | + +User mapping options, all optional: `access_key_id`, `secret_access_key`, +`session_token`, `username`. + +### Where credentials come from + +In this order, and the first that applies wins: + +1. the user mapping for the querying user; +2. the `PUBLIC` user mapping for that server; +3. whatever the host already grants -- environment variables, an instance + profile, a shared credentials file, a web identity token. + +A volume with no user mapping is therefore not a misconfiguration: it is how +an instance profile is meant to be used. Where none of the three yields a +credential, the request is made unsigned and the service refuses it. + +On a host that is not on EC2, the default chain will try the instance metadata +service and wait for it to time out. Set `AWS_EC2_METADATA_DISABLED=true` in +the server's environment to skip that. + +### `file://` volumes + +A `file://` volume is a directory, and every segment reads and writes it +directly. It is only correct if that directory is the *same* directory on +every host -- a network filesystem, or a cluster filesystem. A path that +happens to exist on each host separately will produce a table whose files +exist in several places and nowhere in full. Nothing checks this; it is +yours to arrange. + +## Building with S3 support + +S3 needs the AWS SDK for C++. No distribution packages it, so unless you are +using an image that already has it, build it once: + +```sh +git clone --depth 1 --branch 1.11.844 --recurse-submodules --shallow-submodules \ + https://github.com/aws/aws-sdk-cpp.git +cmake -S aws-sdk-cpp -B aws-sdk-cpp/build -GNinja \ + -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=/opt/datalake \ + -DBUILD_ONLY="s3;sts" -DBUILD_SHARED_LIBS=OFF \ + -DCMAKE_POSITION_INDEPENDENT_CODE=ON \ + -DENABLE_TESTING=OFF -DUSE_OPENSSL=ON +ninja -C aws-sdk-cpp/build install +``` + +It needs libcurl, OpenSSL and zlib headers, and takes well under a minute to +compile; the clone is the slow part. 1.11.844 is the version this extension +is tested against, and 1.11 is the minimum. + +The extension finds it under `/usr/local` or `/opt/datalake`, or wherever +`AWS_SDK_PREFIX` says: + +```sh +make USE_PGXS=1 AWS_SDK_PREFIX=/opt/datalake install +``` + +Without the SDK the extension still builds -- the build says so -- and opening +an `s3://` location then fails with a message telling you to rebuild. Naming +a prefix that has no SDK in it is an error rather than a silent fallback. + +Arrow itself needs no S3 support: this extension does not use Arrow's S3 +filesystem, which is just as well, because no RPM of Arrow is built with it. + +## Writing a storage backend + +A backend is a shared library that answers one question: given a location and +its options, which `arrow::fs::FileSystem` reads and writes it. Everything +else -- opening files, listing, error classification, memory accounting -- is +the extension's side of the boundary. + +`make install` puts five headers under +`$(pg_config --includedir-server)/extension/datalake_fdw/`, and they are all a +backend includes: + +```cpp +#include + +#include "storage_backend.h" +#include "storage_backend_register.h" + +extern "C" { PG_MODULE_MAGIC; void _PG_init(void); } + +static arrow::Result +mount_mine(const DatalakeLocation *location, const DlKeyValue *kv, int nkv, + const DatalakeStorageHost *host) +{ + DatalakeMountedFs mounted; + + mounted.fs = std::make_shared( + arrow::io::IOContext(dl_storage_host_pool(host))); + mounted.root = location->path_prefix; + return mounted; +} + +static const DatalakeStorageBackend mine = { + DL_STORAGE_ABI_VERSION, sizeof(DatalakeStorageBackend), "mine", + ARROW_VERSION_STRING, DL_STORAGE_ABI_FINGERPRINT, mount_mine, NULL, NULL +}; + +void +_PG_init(void) +{ + DlErrCode rc = datalake_storage_register(&mine); + + if (rc != DL_OK) + elog(ERROR, "could not register the \"mine\" storage backend"); +} +``` + +Put the library in `shared_preload_libraries`. The order does not matter: +registering pulls in `datalake_fdw` if it is not loaded yet. Registration +only happens during preload, so that every backend process agrees on which +schemes exist. + +What the extension calls on the filesystem it gets back, and nothing else: + +* `GetFileInfo(path)` and `GetFileInfo(FileSelector)` +* `OpenInputFile`, then `GetSize`, `ReadAt` and `Read` +* `OpenOutputStream`, then `Write`, `Close` and `Abort` +* `DeleteFile` + +Anything else may return `NotImplemented`. + +Three obligations are worth stating, because a backend that gets them wrong +is wrong in ways tests elsewhere will not catch: + +* **`Abort()` removes what this stream created, and only that.** Nothing else + deletes on a stream's behalf, because after a failed write the name may + already belong to another writer. If the stream never created anything -- + an upload that was never started -- abort does nothing. +* **Allocate through `host->pool`.** Arrow's default pool is invisible to + Cloudberry's memory accounting, and a query that allocates outside it is a + query whose memory limit does not apply. +* **Classify failures.** Return `arrow::Status::AlreadyExists` for a name in + use, and for something missing an `IOError` whose text contains + `does not exist` or `No such file or directory` -- those two phrases and + nothing else, because the text also contains the caller's path and a status + number or a service's error code matched inside it would turn an error + *about* a path into an error about the path not existing. Anything else is + reported as an I/O error. + +`abi_fingerprint` is checked at registration: the compiler's major version, +libstdc++'s dual-ABI setting and the Arrow version have to match the ones the +extension was built with, because `std::shared_ptr` and `arrow::Result` cross +the boundary by value. Build a backend with the same toolchain and Arrow +package as the extension. + +## Known limits + +* The fingerprint catches the mismatches that occur in practice, not every + possible one. A backend built against a different C++ runtime can still + register and then misbehave. +* Registering from inside a backend process instead of during preload affects + only that process, and is not supported. +* Between checking that a name is free and creating it, another writer can + take it. Iceberg's file names are unique by construction, so this does not + arise there. +* An upload abandoned by a crashed backend leaves its parts behind. A bucket + lifecycle rule that expires incomplete multipart uploads is the usual answer. +* Reading and writing through `s3://` costs a backend process about 15 MiB of + resident memory that `gp_vmem_protect_limit` does not see: the SDK client, + its connection and one part buffer are allocated by the SDK itself rather + than through the tracked pool. Measured against the same file written + locally, that overhead stays flat as the object grows -- 7.9 MiB for a 2 MB + file, 14.8 MiB for a 149 MB one -- so it is a per-process constant, not a + cost per byte. diff --git a/contrib/datalake_fdw/datalake_fdw_test--1.0.sql b/contrib/datalake_fdw/datalake_fdw_test--1.0.sql index 9171dbcd86f..efa82bbe749 100644 --- a/contrib/datalake_fdw/datalake_fdw_test--1.0.sql +++ b/contrib/datalake_fdw/datalake_fdw_test--1.0.sql @@ -39,17 +39,46 @@ CREATE FUNCTION datalake_parquet_write(path text, query text, row_group_size int DEFAULT 0, - compression text DEFAULT '') -RETURNS bigint AS 'MODULE_PATHNAME' LANGUAGE C STRICT VOLATILE; + compression text DEFAULT '', + volume text DEFAULT NULL) +-- Not STRICT: volume defaults to NULL, and a strict function would answer NULL +-- rather than run. The required arguments are checked in C instead. +RETURNS bigint AS 'MODULE_PATHNAME' LANGUAGE C VOLATILE; -REVOKE EXECUTE ON FUNCTION datalake_parquet_write(text, text, int, text) FROM PUBLIC; +REVOKE EXECUTE ON FUNCTION datalake_parquet_write(text, text, int, text, text) + FROM PUBLIC; -- field_ids names, for each column of the definition list, the Iceberg field -- id it is read from; empty reads the file as it is, column for column. CREATE FUNCTION datalake_parquet_read(path text, first_row_group int DEFAULT 0, n_row_groups int DEFAULT 0, - field_ids int[] DEFAULT '{}') -RETURNS SETOF record AS 'MODULE_PATHNAME' LANGUAGE C STRICT EXECUTE ON COORDINATOR; + field_ids int[] DEFAULT '{}', + volume text DEFAULT NULL) +RETURNS SETOF record AS 'MODULE_PATHNAME' LANGUAGE C EXECUTE ON COORDINATOR; -REVOKE EXECUTE ON FUNCTION datalake_parquet_read(text, int, int, int[]) FROM PUBLIC; +REVOKE EXECUTE ON FUNCTION datalake_parquet_read(text, int, int, int[], text) + FROM PUBLIC; + +-- Test-only storage contract functions. +CREATE FUNCTION datalake_storage_write_text(uri text, content text, + kv text[] DEFAULT NULL) +RETURNS bigint AS 'MODULE_PATHNAME' LANGUAGE C VOLATILE; +CREATE FUNCTION datalake_storage_read_text(uri text, kv text[] DEFAULT NULL) +RETURNS text AS 'MODULE_PATHNAME' LANGUAGE C VOLATILE; +CREATE FUNCTION datalake_storage_list(uri text, kv text[] DEFAULT NULL) +RETURNS SETOF text AS 'MODULE_PATHNAME' +LANGUAGE C VOLATILE EXECUTE ON COORDINATOR; +CREATE FUNCTION datalake_storage_delete(uri text, kv text[] DEFAULT NULL) +RETURNS boolean AS 'MODULE_PATHNAME' LANGUAGE C VOLATILE; +CREATE FUNCTION datalake_storage_probe(scheme text) +RETURNS text AS 'MODULE_PATHNAME' LANGUAGE C VOLATILE; +CREATE FUNCTION datalake_storage_register_bad(kind text) +RETURNS text AS 'MODULE_PATHNAME' LANGUAGE C VOLATILE; + +REVOKE EXECUTE ON FUNCTION datalake_storage_write_text(text, text, text[]) FROM PUBLIC; +REVOKE EXECUTE ON FUNCTION datalake_storage_read_text(text, text[]) FROM PUBLIC; +REVOKE EXECUTE ON FUNCTION datalake_storage_list(text, text[]) FROM PUBLIC; +REVOKE EXECUTE ON FUNCTION datalake_storage_delete(text, text[]) FROM PUBLIC; +REVOKE EXECUTE ON FUNCTION datalake_storage_probe(text) FROM PUBLIC; +REVOKE EXECUTE ON FUNCTION datalake_storage_register_bad(text) FROM PUBLIC; diff --git a/contrib/datalake_fdw/exports.txt b/contrib/datalake_fdw/exports.txt index e89c850b512..08ac1ed74cb 100644 --- a/contrib/datalake_fdw/exports.txt +++ b/contrib/datalake_fdw/exports.txt @@ -30,6 +30,7 @@ pg_finfo_iceberg_catalog_fdw_validator iceberg_catalog_fdw_validator pg_finfo_iceberg_volume_fdw_validator iceberg_volume_fdw_validator +datalake_register_storage_backend # datalake_fdw_test: not part of what this module offers, but a SQL-callable # function has to be found by name in the library like any other. @@ -37,3 +38,15 @@ pg_finfo_datalake_parquet_write datalake_parquet_write pg_finfo_datalake_parquet_read datalake_parquet_read +pg_finfo_datalake_storage_write_text +datalake_storage_write_text +pg_finfo_datalake_storage_read_text +datalake_storage_read_text +pg_finfo_datalake_storage_list +datalake_storage_list +pg_finfo_datalake_storage_delete +datalake_storage_delete +pg_finfo_datalake_storage_probe +datalake_storage_probe +pg_finfo_datalake_storage_register_bad +datalake_storage_register_bad diff --git a/contrib/datalake_fdw/src/am_iceberg/pg_iceberg_extensible.c b/contrib/datalake_fdw/src/am_iceberg/pg_iceberg_extensible.c index 9eec658aa4f..a3302905556 100644 --- a/contrib/datalake_fdw/src/am_iceberg/pg_iceberg_extensible.c +++ b/contrib/datalake_fdw/src/am_iceberg/pg_iceberg_extensible.c @@ -62,6 +62,8 @@ PG_MODULE_MAGIC; +extern DlErrCode datalake_register_test_storage_backend(void); + static ProcessUtility_hook_type prev_ProcessUtility_hook; static bool iceberg_is_effective_am(const char *accessMethod); @@ -1010,6 +1012,8 @@ pg_iceberg_ProcessUtility(PlannedStmt *pstmt, void _PG_init(void) { + DlErrCode test_backend_rc; + if (!process_shared_preload_libraries_in_progress) ereport(ERROR, (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), @@ -1021,6 +1025,10 @@ _PG_init(void) pg_iceberg_register_reloptions(); DatalakeRegisterMetaEngines(); datalake_register_storage_backends(); + test_backend_rc = datalake_register_test_storage_backend(); + if (test_backend_rc != DL_OK && test_backend_rc != DL_ERR_ALREADY_EXISTS) + dl_error_report(ERROR, test_backend_rc, + "register dltest storage backend"); prev_ProcessUtility_hook = ProcessUtility_hook; ProcessUtility_hook = pg_iceberg_ProcessUtility; diff --git a/contrib/datalake_fdw/src/am_iceberg/pg_iceberg_options.c b/contrib/datalake_fdw/src/am_iceberg/pg_iceberg_options.c index 6475546223c..16973344c3c 100644 --- a/contrib/datalake_fdw/src/am_iceberg/pg_iceberg_options.c +++ b/contrib/datalake_fdw/src/am_iceberg/pg_iceberg_options.c @@ -31,6 +31,7 @@ #include "access/relation.h" #include "access/reloptions.h" #include "am_iceberg/pg_iceberg_options.h" +#include "common/backend_registry.h" #include "catalog/dependency.h" #include "catalog/objectaddress.h" #include "catalog/pg_class.h" @@ -516,6 +517,64 @@ s3_bucket_char(char ch) return s3_bucket_alnum(ch) || ch == '.' || ch == '-'; } +/* + * A form of the URI that is safe to put in an error message. + * + * A rejected URI is quoted back so the user can see what was wrong with it, + * but the parts this parser rejects are exactly the parts that carry secrets: + * a query string holds a presigned signature, userinfo holds a password. So + * those are reported as present rather than reproduced. + */ +char * +pg_iceberg_redacted_location_uri(const char *uri) +{ + const char *authority; + const char *cut; + StringInfoData safe; + + if (uri == NULL) + return pstrdup("(null)"); + + initStringInfo(&safe); + + authority = strstr(uri, "://"); + authority = authority == NULL ? uri : authority + 3; + + /* Everything up to the authority, then the authority without userinfo. */ + appendBinaryStringInfo(&safe, uri, (int) (authority - uri)); + cut = strpbrk(authority, "/?#"); + { + Size authority_len = cut == NULL ? + strlen(authority) : (Size) (cut - authority); + const char *at = memchr(authority, '@', authority_len); + + if (at != NULL) + { + appendStringInfoString(&safe, "***@"); + appendBinaryStringInfo(&safe, at + 1, + (int) (authority_len - (at + 1 - authority))); + } + else + appendBinaryStringInfo(&safe, authority, (int) authority_len); + } + + /* Then the path, with any query or fragment named but not repeated. */ + if (cut != NULL) + { + const char *tail = strpbrk(cut, "?#"); + + if (tail == NULL) + appendStringInfoString(&safe, cut); + else + { + appendBinaryStringInfo(&safe, cut, (int) (tail - cut)); + appendStringInfoString(&safe, *tail == '?' ? "?***" : "#***"); + } + } + + return safe.data; +} + DlErrCode pg_iceberg_parse_location(const char *uri, const char *endpoint, const char *region, DatalakeLocation *out, @@ -528,7 +587,9 @@ pg_iceberg_parse_location(const char *uri, const char *endpoint, Size authority_len; Size path_len; bool is_s3; + bool is_file; Size i; + char *safe_uri; Assert(out != NULL); memset(out, 0, sizeof(*out)); @@ -539,72 +600,98 @@ pg_iceberg_parse_location(const char *uri, const char *endpoint, return invalid_location(errdetail, pstrdup("location URI is null")); + safe_uri = pg_iceberg_redacted_location_uri(uri); + scheme_end = strstr(uri, "://"); if (scheme_end == NULL) return invalid_location(errdetail, psprintf("location URI \"%s\" is missing \"://\"", - uri)); + safe_uri)); scheme_len = scheme_end - uri; is_s3 = scheme_len == strlen(DATALAKE_ICEBERG_VOLUME_SERVER_TYPE_S3) && strncmp(uri, DATALAKE_ICEBERG_VOLUME_SERVER_TYPE_S3, scheme_len) == 0; - if (!is_s3 && - !(scheme_len == strlen(DATALAKE_ICEBERG_VOLUME_SERVER_TYPE_HDFS) && - strncmp(uri, DATALAKE_ICEBERG_VOLUME_SERVER_TYPE_HDFS, scheme_len) == 0)) - return invalid_location(errdetail, - psprintf("location URI \"%s\" has unsupported scheme; expected %s or %s", - uri, - DATALAKE_ICEBERG_VOLUME_SERVER_TYPE_S3, - DATALAKE_ICEBERG_VOLUME_SERVER_TYPE_HDFS)); + is_file = scheme_len == strlen("file") && + strncmp(uri, "file", scheme_len) == 0; + + /* + * Two schemes have rules of their own below; any other is acceptable + * exactly when something can read it. A fixed list here would mean a + * third party could register a backend and still have no way to name a + * volume that uses it, which would make the extension contract a promise + * the parser breaks. + */ + if (!is_s3 && !is_file) + { + char *scheme = pnstrdup(uri, scheme_len); + + if (!datalake_storage_scheme_registered(scheme)) + return invalid_location(errdetail, + psprintf("location URI \"%s\" names storage \"%s\", which no backend is registered for", + safe_uri, scheme)); + } + /* + * These three say what is wrong without repeating the URI. A query string + * carries a presigned signature and userinfo carries a password, and an + * error message is read in a log by people the credential was not issued + * to. The caller still reports the value it was given, which is the DDL + * the user just typed, so nothing is lost in diagnosing a typo. + */ if (strchr(uri, '?') != NULL) return invalid_location(errdetail, - psprintf("location URI \"%s\" must not contain a query", - uri)); + pstrdup("location URI must not contain a query")); if (strchr(uri, '#') != NULL) return invalid_location(errdetail, - psprintf("location URI \"%s\" must not contain a fragment", - uri)); + pstrdup("location URI must not contain a fragment")); authority_start = scheme_end + 3; path_start = strchr(authority_start, '/'); authority_len = path_start == NULL ? strlen(authority_start) : (Size) (path_start - authority_start); - if (authority_len == 0) + if (is_s3 && authority_len == 0) return invalid_location(errdetail, psprintf("location URI \"%s\" has an empty authority", - uri)); + safe_uri)); + /* A scheme a backend brought is addressed like file: a path, no host. */ + if (!is_s3 && authority_len != 0) + return invalid_location(errdetail, + psprintf("location URI \"%s\" must have an empty authority", + safe_uri)); + if (!is_s3 && (path_start == NULL || path_start[0] != '/')) + return invalid_location(errdetail, + psprintf("location URI \"%s\" must have an absolute path", + safe_uri)); if (memchr(authority_start, '@', authority_len) != NULL) return invalid_location(errdetail, - psprintf("location URI \"%s\" must not contain userinfo", - uri)); + pstrdup("location URI must not contain userinfo")); if (is_s3) { if (authority_len < 3 || authority_len > 63) return invalid_location(errdetail, psprintf("s3 bucket in location URI \"%s\" must be 3 to 63 characters", - uri)); + safe_uri)); if (!s3_bucket_alnum(authority_start[0]) || !s3_bucket_alnum(authority_start[authority_len - 1])) return invalid_location(errdetail, psprintf("s3 bucket in location URI \"%s\" must start and end with a lowercase letter or digit", - uri)); + safe_uri)); for (i = 0; i < authority_len; i++) { if (!s3_bucket_char(authority_start[i])) return invalid_location(errdetail, psprintf("s3 bucket in location URI \"%s\" contains an invalid character", - uri)); + safe_uri)); } } path_len = path_start == NULL ? 0 : strlen(path_start); - while (path_len > 0 && path_start[path_len - 1] == '/') + while (path_len > (is_file ? 1 : 0) && path_start[path_len - 1] == '/') path_len--; - out->schema_version = DATALAKE_LOCATION_SCHEMA_VERSION; + out->abi_version = DATALAKE_LOCATION_ABI_VERSION; out->scheme = pnstrdup(uri, scheme_len); out->authority = pnstrdup(authority_start, authority_len); out->path_prefix = path_len == 0 ? diff --git a/contrib/datalake_fdw/src/am_iceberg/pg_iceberg_options.h b/contrib/datalake_fdw/src/am_iceberg/pg_iceberg_options.h index d081ffacdb0..f995f6c13dd 100644 --- a/contrib/datalake_fdw/src/am_iceberg/pg_iceberg_options.h +++ b/contrib/datalake_fdw/src/am_iceberg/pg_iceberg_options.h @@ -144,6 +144,13 @@ extern Oid pg_iceberg_am_oid(void); extern MetaKv *pg_iceberg_resolve_credentials(Oid serverid, Oid auth_userid, int *n_props); extern void pg_iceberg_check_server_usage(Oid serverid); +/* + * A form of a location URI that is safe to quote back in an error message: + * the parts that carry secrets -- userinfo, a query string -- are reported as + * present rather than reproduced. Anything that echoes a base_path a user + * typed has to go through this, message and detail alike. + */ +extern char *pg_iceberg_redacted_location_uri(const char *uri); extern DlErrCode pg_iceberg_parse_location(const char *uri, const char *endpoint, const char *region, diff --git a/contrib/datalake_fdw/src/common/backend_registry.cpp b/contrib/datalake_fdw/src/common/backend_registry.cpp index 78fb04a5e33..c557c435939 100644 --- a/contrib/datalake_fdw/src/common/backend_registry.cpp +++ b/contrib/datalake_fdw/src/common/backend_registry.cpp @@ -18,7 +18,7 @@ * under the License. * * backend_registry.cpp - * Registry of the storage backends, one per protocol. + * Process-local registry shared with storage backend plugins. * * IDENTIFICATION * contrib/datalake_fdw/src/common/backend_registry.cpp @@ -26,97 +26,313 @@ *------------------------------------------------------------------------- */ +#include +#include +#include + +#include +#include + +#include "common/storage_backend.h" #include "common/dl_pg_api.h" -#include +extern "C" +{ +#include "fmgr.h" +#include "miscadmin.h" +#include "storage/ipc.h" +#include "utils/memutils.h" +} #include "common/backend_registry.h" #include "common/dl_wrappers.h" -typedef struct DatalakeStorageBackend +#define DL_STORAGE_RENDEZVOUS_NAME "datalake_storage_registry_v1" +#define DL_STORAGE_V1_MIN_SIZE \ + (offsetof(DatalakeStorageBackend, finalize) + \ + sizeof(((DatalakeStorageBackend *) 0)->finalize)) + +typedef struct DatalakeStorageBackendEntry { - const char *scheme; - const struct DatalakeStorageOps *ops; -} DatalakeStorageBackend; + const DatalakeStorageBackend *backend; + bool initialized; + struct DatalakeStorageBackendEntry *next; +} DatalakeStorageBackendEntry; -/* Room for s3 and hdfs, plus space to grow without revisiting this. */ -static DatalakeStorageBackend storage_backends[4]; -static int nstorage_backends; +typedef struct DatalakeStorageRegistry +{ + DatalakeStorageBackendEntry *backends; + void *wrappers; /* reserved for a future wrapper chain */ + bool finalizer_registered; +} DatalakeStorageRegistry; +extern DlErrCode datalake_register_local_backend(void); extern DlErrCode datalake_register_s3_backend(void); -static bool -storage_ops_are_complete(const struct DatalakeStorageOps *ops) +static DatalakeStorageRegistry * +storage_registry(bool create) { - /* - * A partially filled table would turn into a null call at the first - * operation the backend forgot, so refuse it at registration instead. - */ - return ops != NULL && - ops->fs_open != NULL && - ops->fs_close != NULL && - ops->fs_list != NULL && - ops->file_open != NULL && - ops->file_read != NULL && - ops->file_write != NULL && - ops->file_close != NULL && - ops->file_abort != NULL; + void **slot = NULL; + + /* Both calls can allocate and therefore must not longjmp through C++. */ + DL_WRAP_START; + { + slot = find_rendezvous_variable(DL_STORAGE_RENDEZVOUS_NAME); + if (create && *slot == NULL) + *slot = MemoryContextAllocZero(TopMemoryContext, + sizeof(DatalakeStorageRegistry)); + } + DL_WRAP_END; + + return slot == NULL ? NULL : + static_cast(*slot); } -DlErrCode -datalake_register_storage_backend(const char *scheme, - const struct DatalakeStorageOps *ops) +static void +storage_backends_finalize(int code, Datum arg) { - int i; + DatalakeStorageRegistry *registry = NULL; + DatalakeStorageBackendEntry *entry; + + (void) code; + (void) arg; + + try + { + registry = storage_registry(false); + } + catch (...) + { + /* Process exit is a cleanup boundary: never throw or ereport here. */ + return; + } - if (scheme == NULL || scheme[0] == '\0' || !storage_ops_are_complete(ops)) - return DL_ERR_INVALID_OPTION; + if (registry == NULL) + return; - for (i = 0; i < nstorage_backends; i++) + for (entry = registry->backends; entry != NULL; entry = entry->next) { - if (strcmp(storage_backends[i].scheme, scheme) == 0) - return DL_ERR_ALREADY_EXISTS; + if (entry->initialized && entry->backend->finalize != NULL) + { + try + { + DL_WRAP_START; + { + elog(DEBUG1, "datalake_fdw: finalizing storage backend \"%s\"", + entry->backend->uri_scheme); + } + DL_WRAP_END; + entry->backend->finalize(); + } + catch (...) + { + /* One plugin must not prevent the remaining finalizers. */ + } + } + entry->initialized = false; } +} + +static void +set_registration_error(const char *message) +{ + dl_error_set(DL_ERR_INVALID_OPTION, "register storage backend", NULL, + message); +} + +extern "C" __attribute__((visibility("default"))) DlErrCode +datalake_register_storage_backend(const DatalakeStorageBackend *backend) +{ + DlErrCode rc = DL_ERR_INTERNAL; - if (nstorage_backends >= (int) lengthof(storage_backends)) - return DL_ERR_INTERNAL; + DL_ABI_GUARD_BEGIN + { + DatalakeStorageRegistry *registry; + DatalakeStorageBackendEntry *entry; + char message[DL_ERR_MSG_LEN]; - storage_backends[nstorage_backends].scheme = scheme; - storage_backends[nstorage_backends].ops = ops; - nstorage_backends++; + dl_error_reset(); + if (backend == NULL) + { + set_registration_error("expected a non-null storage backend, got null"); + rc = DL_ERR_INVALID_OPTION; + } + else if (backend->abi_version != DL_STORAGE_ABI_VERSION) + { + snprintf(message, sizeof(message), + "storage backend ABI version mismatch: expected %u, got %u", + DL_STORAGE_ABI_VERSION, backend->abi_version); + set_registration_error(message); + rc = DL_ERR_INVALID_OPTION; + } + else if (backend->struct_size < DL_STORAGE_V1_MIN_SIZE) + { + snprintf(message, sizeof(message), + "storage backend struct size mismatch: expected at least %zu, got %u", + (size_t) DL_STORAGE_V1_MIN_SIZE, backend->struct_size); + set_registration_error(message); + rc = DL_ERR_INVALID_OPTION; + } + else if (backend->arrow_version == NULL || + strcmp(backend->arrow_version, ARROW_VERSION_STRING) != 0) + { + snprintf(message, sizeof(message), + "storage backend Arrow version mismatch: expected \"%s\", got \"%s\"", + ARROW_VERSION_STRING, + backend->arrow_version == NULL ? "(null)" : backend->arrow_version); + set_registration_error(message); + rc = DL_ERR_INVALID_OPTION; + } + else if (backend->abi_fingerprint == NULL || + strcmp(backend->abi_fingerprint, + DL_STORAGE_ABI_FINGERPRINT) != 0) + { + snprintf(message, sizeof(message), + "storage backend ABI fingerprint mismatch: expected \"%s\", got \"%s\"", + DL_STORAGE_ABI_FINGERPRINT, + backend->abi_fingerprint == NULL ? + "(null)" : backend->abi_fingerprint); + set_registration_error(message); + rc = DL_ERR_INVALID_OPTION; + } + else if (backend->uri_scheme == NULL || backend->uri_scheme[0] == '\0' || + backend->mount == NULL) + { + set_registration_error("expected a scheme and mount function, got an incomplete storage backend"); + rc = DL_ERR_INVALID_OPTION; + } + else + { + registry = storage_registry(true); + for (entry = registry->backends; entry != NULL; entry = entry->next) + { + if (strcmp(entry->backend->uri_scheme, backend->uri_scheme) == 0) + break; + } - return DL_OK; + if (entry != NULL) + { + snprintf(message, sizeof(message), + "storage backend scheme expected to be unique, got duplicate \"%s\"", + backend->uri_scheme); + dl_error_set(DL_ERR_ALREADY_EXISTS, + "register storage backend", NULL, message); + rc = DL_ERR_ALREADY_EXISTS; + } + else + { + DL_WRAP_START; + { + entry = static_cast( + MemoryContextAllocZero(TopMemoryContext, sizeof(*entry))); + } + DL_WRAP_END; + entry->backend = backend; + entry->next = registry->backends; + registry->backends = entry; + rc = DL_OK; + } + } + } + DL_ABI_GUARD_END(rc, "register_storage_backend"); + + return rc; } -const struct DatalakeStorageOps * +const DatalakeStorageBackend * datalake_lookup_storage_backend(const char *scheme) { - int i; + DatalakeStorageRegistry *registry = storage_registry(false); + DatalakeStorageBackendEntry *entry; - if (scheme == NULL) + if (registry == NULL || scheme == NULL) return NULL; - for (i = 0; i < nstorage_backends; i++) + for (entry = registry->backends; entry != NULL; entry = entry->next) { - if (strcmp(storage_backends[i].scheme, scheme) == 0) - return storage_backends[i].ops; + if (strcmp(entry->backend->uri_scheme, scheme) == 0) + return entry->backend; } - return NULL; } +arrow::Status +datalake_initialize_storage_backend(const DatalakeStorageBackend *backend) +{ + DatalakeStorageRegistry *registry = storage_registry(false); + DatalakeStorageBackendEntry *entry; + + if (registry == NULL || backend == NULL) + return arrow::Status::Invalid("storage backend is not registered"); + + for (entry = registry->backends; entry != NULL; entry = entry->next) + { + if (entry->backend != backend) + continue; + if (entry->initialized) + return arrow::Status::OK(); + + arrow::Status status = arrow::Status::OK(); + + Assert(MyProcPid != PostmasterPid); + if (!registry->finalizer_registered) + { + DL_WRAP_START; + { + on_proc_exit(storage_backends_finalize, (Datum) 0); + } + DL_WRAP_END; + registry->finalizer_registered = true; + } + + if (backend->initialize != NULL) + status = backend->initialize(); + + if (status.ok()) + entry->initialized = true; + return status; + } + + return arrow::Status::Invalid("storage backend is not registered"); +} + +extern "C" bool +datalake_storage_scheme_registered(const char *scheme) +{ + bool found = false; + + /* Asked from C, on a path that must not throw. */ + try + { + found = datalake_lookup_storage_backend(scheme) != NULL; + } + catch (...) + { + found = false; + } + return found; +} + extern "C" void datalake_register_storage_backends(void) { + DlErrCode rc = DL_ERR_INTERNAL; + DL_TRY { - DlErrCode rc = datalake_register_s3_backend(); + rc = datalake_register_local_backend(); + } + DL_CATCH_END(); + + if (rc != DL_OK && rc != DL_ERR_ALREADY_EXISTS) + dl_error_report(ERROR, rc, "register file storage backend"); - /* Registering twice is harmless; anything else is a coding error. */ - if (rc != DL_OK && rc != DL_ERR_ALREADY_EXISTS) - ereport(ERROR, - (errmsg("datalake_fdw: could not register the s3 storage backend: %s", - dl_err_message(rc)))); + DL_TRY + { + rc = datalake_register_s3_backend(); } DL_CATCH_END(); + + if (rc != DL_OK && rc != DL_ERR_ALREADY_EXISTS) + dl_error_report(ERROR, rc, "register s3 storage backend"); } diff --git a/contrib/datalake_fdw/src/common/backend_registry.h b/contrib/datalake_fdw/src/common/backend_registry.h index 5432502ed42..78fcc326dcc 100644 --- a/contrib/datalake_fdw/src/common/backend_registry.h +++ b/contrib/datalake_fdw/src/common/backend_registry.h @@ -18,7 +18,7 @@ * under the License. * * backend_registry.h - * Registry of the storage backends, one per protocol. + * Internal access to the storage backend registry. * * IDENTIFICATION * contrib/datalake_fdw/src/common/backend_registry.h @@ -29,69 +29,27 @@ #ifndef BACKEND_REGISTRY_H #define BACKEND_REGISTRY_H -#include - -#include "common/file_system_wrapper.h" - #ifdef __cplusplus - -/* - * One storage protocol's implementation of the facade in - * common/file_system_wrapper.h. The operations mirror it one for one, so a - * backend is written against the same contract its callers see. - */ -struct DatalakeStorageOps -{ - DlErrCode (*fs_open) (const DatalakeLocation *location, - const DlKeyValue *credentials, int ncredentials, - DatalakeFileSystem *fs_out); - void (*fs_close) (DatalakeFileSystem fs); /* releases fs */ - DlErrCode (*fs_list) (DatalakeFileSystem fs, const char *prefix, - char ***names_out, int *nnames_out); - DlErrCode (*file_open) (DatalakeFileSystem fs, const char *path, - DatalakeFileMode mode, DatalakeFile *file_out); - DlErrCode (*file_read) (DatalakeFile file, void *buffer, int64_t length, - int64_t *nread); - DlErrCode (*file_write) (DatalakeFile file, const void *buffer, - int64_t length); - DlErrCode (*file_close) (DatalakeFile file); /* releases file */ - void (*file_abort) (DatalakeFile file); /* releases file */ -}; - -/* - * Every handle a backend hands out starts with this field, which is how the - * facade finds its way back to the right operations. A handle lives until a - * cleanup entry point consumes it; there is no closed-but-alive state, because - * keeping one would mean either leaking every handle or letting a backend free - * memory the facade still reads. - */ -struct DatalakeFileSystemData -{ - const struct DatalakeStorageOps *ops; -}; - -struct DatalakeFileData -{ - const struct DatalakeStorageOps *ops; -}; - -extern DlErrCode datalake_register_storage_backend(const char *scheme, - const struct DatalakeStorageOps *ops); -extern const struct DatalakeStorageOps *datalake_lookup_storage_backend(const char *scheme); - -#endif /* __cplusplus */ +#include "common/storage_backend.h" +extern const DatalakeStorageBackend *datalake_lookup_storage_backend( + const char *scheme); +extern arrow::Status datalake_initialize_storage_backend( + const DatalakeStorageBackend *backend); +#endif #ifdef __cplusplus extern "C" { #endif +extern void datalake_register_storage_backends(void); + /* - * Registration is an explicit call rather than a static initializer: the order - * static initializers run in a shared module is not something to depend on, - * and _PG_init is where this is meant to happen. + * Whether anything can reach this scheme. The location parser asks, so that + * a volume may name any storage a backend has registered rather than only the + * two this module ships. */ -extern void datalake_register_storage_backends(void); +extern bool datalake_storage_scheme_registered(const char *scheme); #ifdef __cplusplus } diff --git a/contrib/datalake_fdw/src/common/datalake_location.h b/contrib/datalake_fdw/src/common/datalake_location.h index 28bede6f71f..7e73bce279c 100644 --- a/contrib/datalake_fdw/src/common/datalake_location.h +++ b/contrib/datalake_fdw/src/common/datalake_location.h @@ -33,16 +33,17 @@ /* Canonical, versioned location form. URIs are parsed ONCE (options layer); * every backend receives only this struct and must never re-parse URIs. */ -typedef struct DatalakeLocation { - uint32_t schema_version; /* = 1 */ - char *scheme; /* v1 whitelist: "s3" | "hdfs" */ - char *authority; /* s3: bucket (validated); hdfs: namenode[:port] */ - char *path_prefix; /* normalized: always starts with '/', never ends with '/' - * (a bare "/" normalizes to "") */ - char *endpoint; /* optional, may be NULL */ - char *region; /* optional, may be NULL */ +typedef struct DatalakeLocation +{ + uint32_t abi_version; /* = DATALAKE_LOCATION_ABI_VERSION */ + char *scheme; /* v1 whitelist: "s3" | "file" */ + char *authority; /* s3: bucket (validated); file: empty */ + char *path_prefix; /* normalized: starts with '/', no trailing '/' + * except the file root itself */ + char *endpoint; /* optional, may be NULL */ + char *region; /* optional, may be NULL */ } DatalakeLocation; -#define DATALAKE_LOCATION_SCHEMA_VERSION 1 +#define DATALAKE_LOCATION_ABI_VERSION 1 /* Join paths as full = path_prefix + "/" + relative; relative never starts * with '/'. */ diff --git a/contrib/datalake_fdw/src/common/dl_err.c b/contrib/datalake_fdw/src/common/dl_err.c index ce0d94a6618..0aae78de42c 100644 --- a/contrib/datalake_fdw/src/common/dl_err.c +++ b/contrib/datalake_fdw/src/common/dl_err.c @@ -29,6 +29,8 @@ #include "postgres.h" +#include "utils/memutils.h" + #include "common/dl_err.h" #include "utils/guc.h" @@ -90,6 +92,67 @@ dl_error_copy_field(char *dest, Size dest_size, const char *src) strlcpy(dest, src, dest_size); } +/* + * Values that must never appear in what a user or a log is shown. + * + * Credentials arrive as options and end up inside a backend's client, from + * where any number of things can quote them back: an SDK message, an + * exception's what(), a third-party backend's own wording. Scrubbing at each + * of those places means every one of them has to remember to; scrubbing here, + * where every error is recorded, means none of them has to. + * + * The list only grows. A session that has mounted a volume keeps hiding that + * volume's secrets afterwards, which is the safe direction to be wrong in, + * and it stays small because it holds credentials rather than data. + */ +#define DL_MAX_SECRETS 32 +#define DL_MIN_SECRET_LEN 6 /* shorter than this and masking would eat + * ordinary words out of every message */ + +static char *dl_secrets[DL_MAX_SECRETS]; +static int dl_nsecrets; + +void +dl_error_add_secret(const char *value) +{ + int i; + + if (value == NULL || strlen(value) < DL_MIN_SECRET_LEN) + return; + + for (i = 0; i < dl_nsecrets; i++) + { + if (strcmp(dl_secrets[i], value) == 0) + return; + } + + if (dl_nsecrets == DL_MAX_SECRETS) + return; /* a session with 32 distinct secrets is not + * a session whose 33rd needs hiding */ + dl_secrets[dl_nsecrets] = MemoryContextStrdup(TopMemoryContext, value); + dl_nsecrets++; +} + +/* Replaces each secret in place; "***" is shorter than any of them. */ +static void +dl_error_scrub(char *text) +{ + int i; + + for (i = 0; i < dl_nsecrets; i++) + { + const char *secret = dl_secrets[i]; + Size len = strlen(secret); + char *at; + + while ((at = strstr(text, secret)) != NULL) + { + memcpy(at, "***", 3); + memmove(at + 3, at + len, strlen(at + len) + 1); + } + } +} + void dl_error_reset(void) { @@ -114,6 +177,10 @@ dl_error_set(DlErrCode code, const char *operation, const char *type, dl_error_copy_field(dl_error_detail.message, sizeof(dl_error_detail.message), message); dl_error_detail.stack[0] = '\0'; + + /* Whatever produced these, they are about to be shown to somebody. */ + dl_error_scrub(dl_error_detail.type); + dl_error_scrub(dl_error_detail.message); } void diff --git a/contrib/datalake_fdw/src/common/dl_err.h b/contrib/datalake_fdw/src/common/dl_err.h index 7e27ab28545..116606c343f 100644 --- a/contrib/datalake_fdw/src/common/dl_err.h +++ b/contrib/datalake_fdw/src/common/dl_err.h @@ -97,6 +97,14 @@ extern void dl_error_set(DlErrCode code, const char *operation, /* Record the implementation's own numeric code, when it reports one. */ extern void dl_error_set_remote_code(int remote_code); +/* + * Remember a value that must never be shown. Every error recorded afterwards + * has it replaced with "***", whichever layer produced the text -- a backend, + * an SDK, or an exception nobody expected. Values shorter than six characters + * are ignored, because masking those would eat words out of ordinary messages. + */ +extern void dl_error_add_secret(const char *value); + /* Record a stack from the failing implementation. */ extern void dl_error_set_stack(const char *stack); diff --git a/contrib/datalake_fdw/src/common/file_system_wrapper.cpp b/contrib/datalake_fdw/src/common/file_system_wrapper.cpp index c0c8b2d3e51..1cb307fc0af 100644 --- a/contrib/datalake_fdw/src/common/file_system_wrapper.cpp +++ b/contrib/datalake_fdw/src/common/file_system_wrapper.cpp @@ -18,7 +18,7 @@ * under the License. * * file_system_wrapper.cpp - * Storage facade dispatching to the registered backend. + * Storage facade implemented once over Arrow filesystems. * * IDENTIFICATION * contrib/datalake_fdw/src/common/file_system_wrapper.cpp @@ -26,48 +26,313 @@ *------------------------------------------------------------------------- */ -#include "common/dl_pg_api.h" +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include "common/storage_arrow.h" +#include "format/arrow_memory_pool.h" +#include "common/dl_pg_api.h" #include "common/backend_registry.h" #include "common/dl_wrappers.h" #include "common/file_system_wrapper.h" +struct DatalakeFileSystemData +{ + std::shared_ptr fs; + std::string root; + const DatalakeStorageBackend *backend; +}; + +struct DatalakeFileData +{ + std::shared_ptr fs; + std::string path; + std::shared_ptr input; + std::shared_ptr output; +}; + +DlStatusDetail::DlStatusDetail(DlErrCode code, std::string type) + : code_(code), type_(std::move(type)) +{ +} + +const char * +DlStatusDetail::type_id() const +{ + return "datalake::DlStatusDetail"; +} + +std::string +DlStatusDetail::ToString() const +{ + return type_; +} + +DlErrCode +DlStatusDetail::code() const +{ + return code_; +} + +const std::string & +DlStatusDetail::type() const +{ + return type_; +} + /* - * Dispatch only: each call finds the backend registered for the location's - * scheme and hands the work over. Nothing here is reachable from SQL in this - * skeleton, so what the regression suite asserts is the behaviour of the - * layers above. + * The option values worth hiding, handed to the error layer once at mount so + * that everything recorded afterwards has them removed -- including text this + * module never sees, from an SDK or an exception or a third-party backend. + * + * A secret access key and a session token are credentials. An access key id + * is an identifier that appears in request headers and audit records anyway, + * and hiding it costs the reader the one fact that says which credential was + * used -- worse, its value tends to occur inside bucket and prefix names, + * which would then be masked out of every message about them. */ +void +dl_storage_remember_secrets(const DlKeyValue *kv, int nkv) +{ + for (int i = 0; kv != NULL && i < nkv; i++) + { + if (kv[i].key == NULL || kv[i].value == NULL) + continue; + + const std::string key(kv[i].key); + + if (key.find("secret") == std::string::npos && + key.find("token") == std::string::npos && + key.find("password") == std::string::npos) + continue; + + dl_error_add_secret(kv[i].value); + } +} + +/* + * Which DlErrCode a status means, and what to call its class. + * + * A backend that classified the failure itself is believed: only it can know + * that one service's 404 and another's ENOENT are the same answer. For any + * other status this is the fallback, and it reads the whole status rather than + * the message alone -- Arrow puts a failed open's errno in a detail, so the + * words "No such file or directory" are not in the message at all. + */ +static DlErrCode +classify_status(const arrow::Status &status, std::string *type_out) +{ + if (status.detail() != NULL && + strcmp(status.detail()->type_id(), "datalake::DlStatusDetail") == 0) + { + const DlStatusDetail *detail = + static_cast(status.detail().get()); + + *type_out = detail->type(); + return detail->code(); + } + + /* + * Arrow prints "Unknown" for the codes it has no name for, AlreadyExists + * among them, and a class of "Unknown" beside a message that explains + * itself is worse than no class at all. + */ + *type_out = status.CodeAsString(); + if (*type_out == "Unknown") + type_out->clear(); + + if (status.IsNotImplemented()) + return DL_ERR_NOT_SUPPORTED; + if (status.IsAlreadyExists()) + return DL_ERR_ALREADY_EXISTS; + if (status.IsOutOfMemory()) + return DL_ERR_OUT_OF_MEMORY; + if (status.IsInvalid() || status.IsTypeError() || status.IsKeyError()) + return DL_ERR_INVALID_OPTION; + if (status.IsIOError()) + { + /* + * Only phrases a filesystem writes about itself, never a bare status + * number: the text contains the caller's path, and a directory named + * "404" would otherwise turn every error about it into "not found". + * A backend that can tell properly attaches its own classification + * instead of leaving this to guess. + */ + const std::string whole = status.ToString(); + + if (whole.find("No such file or directory") != std::string::npos || + whole.find("does not exist") != std::string::npos) + return DL_ERR_NOT_FOUND; + } + return DL_ERR_IO; +} + +/* + * The same status, carrying its classification, for a caller that hands it on + * rather than reporting it here -- the format layer, which opens files through + * the storage layer but reports errors in its own terms. + */ +arrow::Status +dl_storage_classify(arrow::Status status) +{ + std::string type; + + if (status.ok() || (status.detail() != NULL && + strcmp(status.detail()->type_id(), + "datalake::DlStatusDetail") == 0)) + return status; + + DlErrCode code = classify_status(status, &type); + + return status.WithDetail(std::make_shared(code, type)); +} + +DlErrCode +dl_storage_status_to_err(const arrow::Status &status, const char *operation) +{ + DlErrCode code; + std::string type; + std::string message; + + if (status.ok()) + return DL_OK; + + code = classify_status(status, &type); + if (message.empty()) + message = status.message(); + dl_error_set(code, operation, type.c_str(), message.c_str()); + return code; +} + +bool +dl_storage_path_is_safe(const char *relative) +{ + const char *segment = relative; + + if (relative == NULL || relative[0] == '\0') + return true; /* the mount root itself */ + if (relative[0] == '/') + return false; /* absolute: not relative to the mount */ + + while (segment != NULL) + { + const char *slash = strchr(segment, '/'); + size_t length = slash == NULL ? strlen(segment) : + static_cast(slash - segment); + + if ((length == 1 && segment[0] == '.') || + (length == 2 && segment[0] == '.' && segment[1] == '.')) + return false; + segment = slash == NULL ? NULL : slash + 1; + } + return true; +} + +/* Reports the rejection the caller should return for an unsafe path. */ +static DlErrCode +reject_unsafe_path(const char *operation, const char *relative) +{ + std::string message = "storage path \"" + std::string(relative) + + "\" must be relative to the volume and must not contain \".\" or \"..\""; + + dl_error_set(DL_ERR_INVALID_OPTION, operation, NULL, message.c_str()); + return DL_ERR_INVALID_OPTION; +} + +arrow::fs::FileSystem * +dl_storage_arrow_fs(DatalakeFileSystem fs) +{ + return fs == NULL ? NULL : fs->fs.get(); +} + +std::string +dl_storage_native_path(DatalakeFileSystem fs, const char *relative) +{ + if (fs == NULL || relative == NULL || relative[0] == '\0') + return fs == NULL ? std::string() : fs->root; + if (fs->root.empty()) + return relative; + if (fs->root.back() == '/') + return fs->root + relative; + return fs->root + "/" + relative; +} extern "C" DlErrCode -datalake_fs_open(const DatalakeLocation *location, - const DlKeyValue *credentials, int ncredentials, - DatalakeFileSystem *fs_out) +datalake_fs_open(const DatalakeLocation *location, const DlKeyValue *kv, + int nkv, DatalakeFileSystem *fs_out) { DlErrCode rc = DL_ERR_INTERNAL; DL_ABI_GUARD_BEGIN { - const struct DatalakeStorageOps *ops; - if (fs_out == NULL || location == NULL || location->scheme == NULL || - ncredentials < 0) - rc = DL_ERR_INVALID_OPTION; + nkv < 0 || (nkv > 0 && kv == NULL)) + rc = DL_ARG_ERROR("fs_open"); else { + const DatalakeStorageBackend *backend; + *fs_out = NULL; - ops = datalake_lookup_storage_backend(location->scheme); + dl_error_reset(); + backend = datalake_lookup_storage_backend(location->scheme); + if (backend == NULL) + { + std::string message = "no storage backend registered for scheme \"" + + std::string(location->scheme) + "\""; - if (ops == NULL) + dl_error_set(DL_ERR_NOT_SUPPORTED, "mount storage", NULL, + message.c_str()); rc = DL_ERR_NOT_SUPPORTED; + } else { - rc = ops->fs_open(location, credentials, ncredentials, fs_out); + arrow::Status status; + + dl_storage_remember_secrets(kv, nkv); + status = datalake_initialize_storage_backend(backend); + + if (!status.ok()) + rc = dl_storage_status_to_err(status, "initialize storage"); + else + { + DatalakeStorageHost host; + + host.struct_size = sizeof(host); + host.pool = DlArrowMemoryPool(); + + auto mounted = backend->mount(location, kv, nkv, &host); - if (rc == DL_OK && *fs_out == NULL) - rc = DL_ERR_INTERNAL; - else if (rc == DL_OK) - (*fs_out)->ops = ops; + if (!mounted.ok()) + rc = dl_storage_status_to_err(mounted.status(), + "mount storage"); + else if (mounted->fs == NULL) + { + dl_error_set(DL_ERR_INTERNAL, "mount storage", NULL, + "storage backend returned a null filesystem"); + rc = DL_ERR_INTERNAL; + } + else + { + std::unique_ptr handle( + new DatalakeFileSystemData()); + + handle->fs = std::move(mounted->fs); + handle->root = std::move(mounted->root); + handle->backend = backend; + *fs_out = handle.release(); + rc = DL_OK; + } + } } } } @@ -81,18 +346,12 @@ datalake_fs_close(DatalakeFileSystem *fs) { DL_CLEANUP_GUARD_BEGIN { - /* - * Clear the caller's handle before releasing it, so that a repeated - * close -- the normal shape of resource-owner cleanup after an error - * that already closed things -- finds nothing to do instead of - * reaching a backend that has freed itself. - */ if (fs != NULL && *fs != NULL) { DatalakeFileSystem doomed = *fs; *fs = NULL; - doomed->ops->fs_close(doomed); + delete doomed; } } DL_CLEANUP_GUARD_END; @@ -106,14 +365,100 @@ datalake_fs_list(DatalakeFileSystem fs, const char *prefix, DL_ABI_GUARD_BEGIN { - if (fs == NULL || prefix == NULL || names_out == NULL || - nnames_out == NULL) - rc = DL_ERR_INVALID_OPTION; + if (fs == NULL || prefix == NULL || names_out == NULL || nnames_out == NULL) + rc = DL_ARG_ERROR("fs_list"); + else if (!dl_storage_path_is_safe(prefix)) + rc = reject_unsafe_path("list storage", prefix); else { + arrow::fs::FileSelector selector; + std::vector paths; + *names_out = NULL; *nnames_out = 0; - rc = fs->ops->fs_list(fs, prefix, names_out, nnames_out); + selector.base_dir = dl_storage_native_path(fs, prefix); + selector.recursive = true; + selector.allow_not_found = true; /* the emptiness rule is above */ + auto infos = fs->fs->GetFileInfo(selector); + + if (!infos.ok()) + rc = dl_storage_status_to_err(infos.status(), "list storage"); + else if (infos->empty()) + { + /* + * Nothing at all under the prefix. Object storage cannot + * tell an empty prefix from one that was never written, and a + * filesystem would answer differently, so the rule is made + * here rather than by each backend: nothing there is nothing + * to list, and a caller that named the wrong prefix is told + * so instead of reading an empty table. + */ + std::string message = prefix[0] == '\0' ? + std::string("nothing is stored at the root of this " + "location") : + "nothing is stored under \"" + std::string(prefix) + "\""; + + dl_error_set(DL_ERR_NOT_FOUND, "list storage", NULL, + message.c_str()); + rc = DL_ERR_NOT_FOUND; + } + else + { + for (const auto &info : *infos) + { + if (info.IsFile()) + paths.push_back(info.path()); + } + std::sort(paths.begin(), paths.end()); + + /* The count leaves through an int, so it has to fit in one. */ + if (paths.size() > static_cast(INT_MAX)) + { + dl_error_set(DL_ERR_IO, "list storage", NULL, + "storage listing has too many entries to return"); + rc = DL_ERR_IO; + } + else + { + char **names = static_cast( + std::calloc(paths.size(), sizeof(char *))); + + if (!paths.empty() && names == NULL) + rc = dl_storage_status_to_err( + arrow::Status::OutOfMemory("allocating storage listing"), + "list storage"); + else + { + size_t i = 0; + + for (; i < paths.size(); i++) + { + names[i] = static_cast( + std::malloc(paths[i].size() + 1)); + if (names[i] == NULL) + break; + std::memcpy(names[i], paths[i].c_str(), + paths[i].size() + 1); + } + + if (i != paths.size()) + { + while (i > 0) + std::free(names[--i]); + std::free(names); + rc = dl_storage_status_to_err( + arrow::Status::OutOfMemory("allocating storage listing"), + "list storage"); + } + else + { + *names_out = names; + *nnames_out = static_cast(paths.size()); + rc = DL_OK; + } + } + } + } } } DL_ABI_GUARD_END(rc, "fs_list"); @@ -121,6 +466,81 @@ datalake_fs_list(DatalakeFileSystem fs, const char *prefix, return rc; } +/* + * The existence rule both openers share. Checking before creating is not what + * makes a write safe -- two writers can still pass the check together, and on + * object storage there is nothing to hold -- it is what turns a name collision + * into an error a user can read instead of a file somebody silently lost. The + * backends enforce the rule where they can: the local one creates with O_EXCL. + */ +static arrow::Status +require_absent(DatalakeFileSystem fs, const std::string &native) +{ + ARROW_ASSIGN_OR_RAISE(auto info, fs->fs->GetFileInfo(native)); + + if (info.type() != arrow::fs::FileType::NotFound) + return arrow::Status::AlreadyExists("\"", native, "\" already exists"); + return arrow::Status::OK(); +} + +arrow::Result> +dl_storage_open_input(DatalakeFileSystem fs, const char *relative) +{ + if (fs == NULL || relative == NULL) + return arrow::Status::Invalid("no file system or path to open"); + if (!dl_storage_path_is_safe(relative)) + return arrow::Status::Invalid("storage path \"", relative, + "\" must be relative to the volume and " + "must not contain \".\" or \"..\""); + auto file = fs->fs->OpenInputFile(dl_storage_native_path(fs, relative)); + + if (!file.ok()) + return dl_storage_classify(file.status()); + return file; +} + +arrow::Result> +dl_storage_open_output(DatalakeFileSystem fs, const char *relative) +{ + if (fs == NULL || relative == NULL) + return arrow::Status::Invalid("no file system or path to create"); + if (!dl_storage_path_is_safe(relative)) + return arrow::Status::Invalid("storage path \"", relative, + "\" must be relative to the volume and " + "must not contain \".\" or \"..\""); + + const std::string native = dl_storage_native_path(fs, relative); + + ARROW_RETURN_NOT_OK(dl_storage_classify(require_absent(fs, native))); + + auto stream = fs->fs->OpenOutputStream(native); + + if (!stream.ok()) + return dl_storage_classify(stream.status()); + return stream; +} + +extern "C" DlErrCode +datalake_file_delete(DatalakeFileSystem fs, const char *path) +{ + DlErrCode rc = DL_ERR_INTERNAL; + + DL_ABI_GUARD_BEGIN + { + if (fs == NULL || path == NULL) + rc = DL_ARG_ERROR("file_delete"); + else if (!dl_storage_path_is_safe(path)) + rc = reject_unsafe_path("delete storage file", path); + else + rc = dl_storage_status_to_err( + fs->fs->DeleteFile(dl_storage_native_path(fs, path)), + "delete storage file"); + } + DL_ABI_GUARD_END(rc, "file_delete"); + + return rc; +} + extern "C" DlErrCode datalake_file_open(DatalakeFileSystem fs, const char *path, DatalakeFileMode mode, DatalakeFile *file_out) @@ -129,19 +549,75 @@ datalake_file_open(DatalakeFileSystem fs, const char *path, DL_ABI_GUARD_BEGIN { - if (file_out == NULL || fs == NULL || path == NULL) - rc = DL_ERR_INVALID_OPTION; - else if (mode != DATALAKE_FILE_READ && mode != DATALAKE_FILE_WRITE) - rc = DL_ERR_INVALID_OPTION; + if (file_out == NULL || fs == NULL || path == NULL || + (mode != DATALAKE_FILE_READ && mode != DATALAKE_FILE_WRITE)) + rc = DL_ARG_ERROR("file_open"); + else if (!dl_storage_path_is_safe(path)) + rc = reject_unsafe_path("open storage file", path); else { + std::string native_path = dl_storage_native_path(fs, path); + auto info = fs->fs->GetFileInfo(native_path); + *file_out = NULL; - rc = fs->ops->file_open(fs, path, mode, file_out); + if (!info.ok()) + rc = dl_storage_status_to_err(info.status(), "inspect storage file"); + else if (mode == DATALAKE_FILE_READ && + info->type() == arrow::fs::FileType::NotFound) + { + std::string message = "storage file \"" + native_path + + "\" does not exist"; - if (rc == DL_OK && *file_out == NULL) - rc = DL_ERR_INTERNAL; - else if (rc == DL_OK) - (*file_out)->ops = fs->ops; + dl_error_set(DL_ERR_NOT_FOUND, "open storage file", NULL, + message.c_str()); + rc = DL_ERR_NOT_FOUND; + } + else if (mode == DATALAKE_FILE_WRITE && + info->type() != arrow::fs::FileType::NotFound) + { + std::string message = "storage file \"" + native_path + + "\" already exists"; + + dl_error_set(DL_ERR_ALREADY_EXISTS, "create storage file", NULL, + message.c_str()); + rc = DL_ERR_ALREADY_EXISTS; + } + else + { + std::unique_ptr handle(new DatalakeFileData()); + + handle->fs = fs->fs; + handle->path = native_path; + if (mode == DATALAKE_FILE_READ) + { + auto input = fs->fs->OpenInputFile(native_path); + + if (!input.ok()) + rc = dl_storage_status_to_err(input.status(), + "open storage file"); + else + { + handle->input = *input; + rc = DL_OK; + } + } + else + { + auto output = fs->fs->OpenOutputStream(native_path); + + if (!output.ok()) + rc = dl_storage_status_to_err(output.status(), + "create storage file"); + else + { + handle->output = *output; + rc = DL_OK; + } + } + + if (rc == DL_OK) + *file_out = handle.release(); + } } } DL_ABI_GUARD_END(rc, "file_open"); @@ -157,12 +633,21 @@ datalake_file_read(DatalakeFile file, void *buffer, int64_t length, DL_ABI_GUARD_BEGIN { - if (file == NULL || nread == NULL || length < 0) - rc = DL_ERR_INVALID_OPTION; + if (file == NULL || file->input == NULL || nread == NULL || length < 0 || + (length > 0 && buffer == NULL)) + rc = DL_ARG_ERROR("file_read"); else { + auto read = file->input->Read(length, buffer); + *nread = 0; - rc = file->ops->file_read(file, buffer, length, nread); + if (!read.ok()) + rc = dl_storage_status_to_err(read.status(), "read storage file"); + else + { + *nread = *read; + rc = DL_OK; + } } } DL_ABI_GUARD_END(rc, "file_read"); @@ -177,10 +662,12 @@ datalake_file_write(DatalakeFile file, const void *buffer, int64_t length) DL_ABI_GUARD_BEGIN { - if (file == NULL || length < 0) - rc = DL_ERR_INVALID_OPTION; + if (file == NULL || file->output == NULL || length < 0 || + (length > 0 && buffer == NULL)) + rc = DL_ARG_ERROR("file_write"); else - rc = file->ops->file_write(file, buffer, length); + rc = dl_storage_status_to_err(file->output->Write(buffer, length), + "write storage file"); } DL_ABI_GUARD_END(rc, "file_write"); @@ -195,18 +682,24 @@ datalake_file_close(DatalakeFile *file) DL_ABI_GUARD_BEGIN { if (file == NULL || *file == NULL) - rc = DL_ERR_INVALID_OPTION; + rc = DL_ARG_ERROR("file_close"); else { - DatalakeFile doomed = *file; + std::unique_ptr doomed(*file); + arrow::Status status; + + *file = NULL; /* - * The handle is consumed even when the close reports an error: - * the backend has released it either way, and there is nothing - * left to retry the close against. + * A stream cleans up after itself: the output stream removes what + * it created, and only that. Deleting by path from here would + * reach an object that some other writer created in the meantime. */ - *file = NULL; - rc = doomed->ops->file_close(doomed); + status = doomed->input != NULL ? doomed->input->Close() : + doomed->output->Close(); + if (!status.ok() && doomed->output != NULL) + (void) doomed->output->Abort(); + rc = dl_storage_status_to_err(status, "close storage file"); } } DL_ABI_GUARD_END(rc, "file_close"); @@ -219,13 +712,15 @@ datalake_file_abort(DatalakeFile *file) { DL_CLEANUP_GUARD_BEGIN { - /* Cleared first, so a repeated abort finds nothing to do. */ if (file != NULL && *file != NULL) { - DatalakeFile doomed = *file; + std::unique_ptr doomed(*file); *file = NULL; - doomed->ops->file_abort(doomed); + if (doomed->output != NULL) + (void) doomed->output->Abort(); + else if (doomed->input != NULL) + (void) doomed->input->Close(); } } DL_CLEANUP_GUARD_END; diff --git a/contrib/datalake_fdw/src/common/file_system_wrapper.h b/contrib/datalake_fdw/src/common/file_system_wrapper.h index 37b6c037d2c..98ed4bb0aaa 100644 --- a/contrib/datalake_fdw/src/common/file_system_wrapper.h +++ b/contrib/datalake_fdw/src/common/file_system_wrapper.h @@ -31,9 +31,9 @@ #include -#include "common/datalake_location.h" -#include "common/dl_err.h" -#include "common/dl_kv.h" +#include "datalake_location.h" +#include "dl_err.h" +#include "dl_kv.h" /* * A file system reached over one storage protocol, and an open file in it. @@ -82,6 +82,13 @@ extern void datalake_fs_close(DatalakeFileSystem *fs); extern DlErrCode datalake_fs_list(DatalakeFileSystem fs, const char *prefix, char ***names_out, int *nnames_out); +/* + * Remove one file. Nothing in the write path calls this -- a stream that + * gives up removes its own work -- but a caller that knows an object is + * finished with can say so. + */ +extern DlErrCode datalake_file_delete(DatalakeFileSystem fs, const char *path); + extern DlErrCode datalake_file_open(DatalakeFileSystem fs, const char *path, DatalakeFileMode mode, DatalakeFile *file_out); diff --git a/contrib/datalake_fdw/src/common/local_file_system.cpp b/contrib/datalake_fdw/src/common/local_file_system.cpp new file mode 100644 index 00000000000..689864d8030 --- /dev/null +++ b/contrib/datalake_fdw/src/common/local_file_system.cpp @@ -0,0 +1,230 @@ +/*------------------------------------------------------------------------- + * + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * + * local_file_system.cpp + * The create-only local storage backend. + * + * IDENTIFICATION + * contrib/datalake_fdw/src/common/local_file_system.cpp + * + *------------------------------------------------------------------------- + */ + +#include +#include +#include +#include + +#include + +#include +#include +#include + +#include "common/local_file_system.h" +#include "common/storage_backend.h" + +extern "C" +{ +#include "postgres.h" +#include "common/file_perm.h" +} + +namespace +{ + +/* + * The descriptor between open() and the point where Arrow takes it over. Any + * error in between has to undo the creation, or an empty file is left behind + * and every later attempt at the same path fails with ALREADY_EXISTS. + */ +class CreatedFile +{ +public: + CreatedFile(int fd, std::string path) : fd_(fd), path_(std::move(path)) {} + + ~CreatedFile() + { + if (fd_ >= 0) + { + (void) close(fd_); + (void) unlink(path_.c_str()); + } + } + + CreatedFile(const CreatedFile &) = delete; + CreatedFile &operator=(const CreatedFile &) = delete; + + int fd() const { return fd_; } + void release() { fd_ = -1; } + +private: + int fd_; + std::string path_; +}; + +/* + * Wraps the Arrow stream so that giving up removes what this writer created + * and nothing else. Only the path this stream created with O_EXCL is ever + * unlinked, which is what keeps a failed writer from destroying a file that + * belongs to somebody else. + */ +class CreateOnlyOutputStream : public arrow::io::OutputStream +{ +public: + CreateOnlyOutputStream(std::shared_ptr file, + std::string path) + : file_(std::move(file)), path_(std::move(path)) + { + } + + /* + * Nothing should reach this without a Close or an Abort, but a C++ + * exception on the way out of a caller can, and what it would leave is an + * empty file that makes every later attempt at the name fail. + */ + ~CreateOnlyOutputStream() override + { + if (!done_) + (void) Abort(); + } + + arrow::Status Close() override + { + if (done_) + return arrow::Status::OK(); + done_ = true; + + arrow::Status status = file_->Close(); + + /* A close that fails leaves a file nobody asked for. */ + if (!status.ok()) + (void) unlink(path_.c_str()); + return status; + } + + /* + * Abort is the caller saying "this object must not exist". Close the + * descriptor, then remove the file this stream created. + */ + arrow::Status Abort() override + { + if (done_) + return arrow::Status::OK(); + done_ = true; + + arrow::Status status = file_->Close(); + + if (unlink(path_.c_str()) != 0 && errno != ENOENT) + return arrow::Status::IOError("could not remove \"", path_, + "\": ", strerror(errno)); + return status; + } + + bool closed() const override { return done_ || file_->closed(); } + + arrow::Result Tell() const override { return file_->Tell(); } + + arrow::Status Write(const void *data, int64_t nbytes) override + { + return file_->Write(data, nbytes); + } + + arrow::Status Write(const std::shared_ptr &data) override + { + return file_->Write(data); + } + + arrow::Status Flush() override { return file_->Flush(); } + +private: + std::shared_ptr file_; + std::string path_; + bool done_ = false; +}; + +} /* namespace */ + +arrow::Result> +LocalCreateOnlyFileSystem::OpenOutputStream( + const std::string &path, + const std::shared_ptr &metadata) +{ + int fd; + + (void) metadata; + fd = open(path.c_str(), O_WRONLY | O_CREAT | O_EXCL, pg_file_create_mode); + if (fd < 0) + { + if (errno == EEXIST) + return arrow::Status::AlreadyExists("\"", path, "\" already exists"); + return arrow::Status::IOError("could not create \"", path, "\": ", + strerror(errno)); + } + + CreatedFile created(fd, path); + auto stream = arrow::io::FileOutputStream::Open(fd); + + if (!stream.ok()) + return stream.status(); /* CreatedFile closes and unlinks */ + + /* + * Built before the guard is disarmed: allocating the wrapper can throw, + * and the file must not survive that either. + */ + auto owned = std::make_shared(*stream, path); + + created.release(); /* the wrapper owns the file now */ + return owned; +} + +static arrow::Result +mount_local(const DatalakeLocation *location, const DlKeyValue *kv, int nkv, + const DatalakeStorageHost *host) +{ + (void) kv; + (void) nkv; + + if (location == NULL || location->path_prefix == NULL) + return arrow::Status::Invalid("file location has no path"); + + DatalakeMountedFs mounted; + + mounted.fs = std::make_shared( + arrow::io::IOContext(dl_storage_host_pool(host))); + mounted.root = location->path_prefix; + return mounted; +} + +static const DatalakeStorageBackend local_storage_backend = { + DL_STORAGE_ABI_VERSION, + sizeof(DatalakeStorageBackend), + "file", + ARROW_VERSION_STRING, + DL_STORAGE_ABI_FINGERPRINT, + mount_local, + NULL, + NULL +}; + +DlErrCode +datalake_register_local_backend(void) +{ + return datalake_register_storage_backend(&local_storage_backend); +} diff --git a/contrib/datalake_fdw/src/common/local_file_system.h b/contrib/datalake_fdw/src/common/local_file_system.h new file mode 100644 index 00000000000..0ff89a40387 --- /dev/null +++ b/contrib/datalake_fdw/src/common/local_file_system.h @@ -0,0 +1,64 @@ +/*------------------------------------------------------------------------- + * + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * + * local_file_system.h + * The create-only local file system, shared with the test backend. + * + * IDENTIFICATION + * contrib/datalake_fdw/src/common/local_file_system.h + * + *------------------------------------------------------------------------- + */ + +#ifndef LOCAL_FILE_SYSTEM_H +#define LOCAL_FILE_SYSTEM_H + +#include +#include + +#include +#include + +/* + * A local file system whose OpenOutputStream creates the file and fails if it + * is already there, and whose streams clean up after themselves. + * + * Arrow's own LocalFileSystem opens output with O_TRUNC, which would let a + * failed write destroy an existing file; this keeps the O_EXCL rule the + * Parquet writer has had since #1951. The returned stream owns what it + * created: Abort(), and a Close() that fails, unlink exactly the path this + * stream created, and nothing else ever deletes on its behalf. + * + * The test backend mounts the same implementation through SubTreeFileSystem, + * so the storage conformance cases exercise one implementation, not two. + */ +class LocalCreateOnlyFileSystem : public arrow::fs::LocalFileSystem +{ +public: + explicit LocalCreateOnlyFileSystem(const arrow::io::IOContext &io_context) + : arrow::fs::LocalFileSystem(io_context) + { + } + + arrow::Result> OpenOutputStream( + const std::string &path, + const std::shared_ptr &metadata) override; +}; + +#endif /* LOCAL_FILE_SYSTEM_H */ diff --git a/contrib/datalake_fdw/src/common/s3_file_system.cpp b/contrib/datalake_fdw/src/common/s3_file_system.cpp index ebb7fadf2ad..3548d6375d1 100644 --- a/contrib/datalake_fdw/src/common/s3_file_system.cpp +++ b/contrib/datalake_fdw/src/common/s3_file_system.cpp @@ -18,7 +18,18 @@ * under the License. * * s3_file_system.cpp - * The S3 storage backend. + * The S3 storage backend, over the AWS SDK for C++. + * + * Arrow can be built with an S3 file system of its own, but no RPM of it is: + * Apache's own spec turns the option off, and so does EPEL, which is every + * Arrow a Rocky or RHEL user can install. So the file system is ours, built + * on the SDK directly, and the rest of the module neither knows nor cares -- + * it sees an arrow::fs::FileSystem like any other backend produces. + * + * Only the synchronous S3Client is used. A PostgreSQL backend is a single + * thread that must stay interruptible and must account for its own memory, so + * the CRT client and the transfer manager, which bring their own thread pools + * and buffers, would both be working against us. * * IDENTIFICATION * contrib/datalake_fdw/src/common/s3_file_system.cpp @@ -26,236 +37,1034 @@ *------------------------------------------------------------------------- */ -#include "common/dl_pg_api.h" +#include +#include +#include +#include +#include +#include -#include "common/backend_registry.h" -#include "common/dl_wrappers.h" +#include +#include +#include +#include -#include +#include "common/storage_backend.h" + +#ifdef DL_HAVE_AWS_SDK + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "common/storage_arrow.h" + +extern "C" +{ +#include "postgres.h" +#include "miscadmin.h" +} + +namespace +{ + +const char *const DL_S3_ALLOC_TAG = "datalake_fdw"; + +/* Big enough to clear S3's 5 MiB minimum for every part but the last. */ +constexpr int64_t DL_S3_PART_SIZE = 8 * 1024 * 1024; + +Aws::SDKOptions sdk_options; + +/* ---------------------------------------------------------------------- + * Errors + * ---------------------------------------------------------------------- */ + +template +bool +is_not_found(const AwsError &error) +{ + return error.GetResponseCode() == Aws::Http::HttpResponseCode::NOT_FOUND || + error.GetErrorType() == Aws::S3::S3Errors::NO_SUCH_KEY || + error.GetErrorType() == Aws::S3::S3Errors::NO_SUCH_BUCKET || + error.GetErrorType() == Aws::S3::S3Errors::RESOURCE_NOT_FOUND; +} /* - * The S3 backend, without an S3 client yet: the shape a backend takes is what - * this file establishes, so that the change adding a real client replaces - * method bodies rather than the structure around them. Every entry point - * reports that the operation is not supported. + * An SDK failure as an Arrow status the facade can classify. The exception + * name travels in the detail, so the user sees "AccessDenied" rather than a + * bare I/O error, and DlErrCode is decided here rather than by matching on + * the text of the message further up. + * + * What goes in the message is the operation, the bucket and the key -- never + * a header or a credential, which is why the SDK's own message is the last + * thing appended and nothing else about the request is. */ -class S3FileSystem +template +arrow::Status +status_from_aws(const char *operation, const std::string &bucket, + const std::string &key, const AwsError &error) +{ + DlErrCode code = is_not_found(error) ? DL_ERR_NOT_FOUND : DL_ERR_IO; + std::string name(error.GetExceptionName().c_str()); + std::string message = std::string(operation) + " s3://" + bucket + "/" + + key + " failed"; + + if (name.empty()) + name = "S3Error"; + if (error.GetMessage().size() > 0) + message += ": " + std::string(error.GetMessage().c_str()); + + return arrow::Status::IOError(message) + .WithDetail(std::make_shared(code, name)); +} + +/* ---------------------------------------------------------------------- + * Paths + * ---------------------------------------------------------------------- */ + +/* The facade hands down "bucket/key"; the SDK wants the two apart. */ +void +split_path(const std::string &path, std::string *bucket, std::string *key) +{ + std::string::size_type slash = path.find('/'); + + if (slash == std::string::npos) + { + *bucket = path; + key->clear(); + return; + } + *bucket = path.substr(0, slash); + *key = path.substr(slash + 1); + while (!key->empty() && key->back() == '/') + key->pop_back(); +} + +/* ---------------------------------------------------------------------- + * Reading + * ---------------------------------------------------------------------- */ + +class S3InputFile : public arrow::io::RandomAccessFile { public: - DlErrCode - Initialize(const DatalakeLocation *location, const DlKeyValue *credentials, - int ncredentials) + S3InputFile(std::shared_ptr client, std::string bucket, + std::string key, int64_t size, arrow::MemoryPool *pool) + : client_(std::move(client)), bucket_(std::move(bucket)), + key_(std::move(key)), size_(size), pool_(pool) { - (void) location; - (void) credentials; - (void) ncredentials; + } - return DL_ERR_NOT_SUPPORTED; + arrow::Status Close() override + { + closed_ = true; + return arrow::Status::OK(); } - DlErrCode - OpenFile(const char *path, DatalakeFileMode mode, DatalakeFile *file_out) + bool closed() const override { return closed_; } + + arrow::Result Tell() const override { - (void) path; - (void) mode; + if (closed_) + return arrow::Status::Invalid("the file is closed"); + return position_; + } + + arrow::Status Seek(int64_t position) override + { + if (closed_) + return arrow::Status::Invalid("the file is closed"); + if (position < 0) + return arrow::Status::Invalid("cannot seek to a negative position"); + position_ = position; + return arrow::Status::OK(); + } + + arrow::Result GetSize() override + { + if (closed_) + return arrow::Status::Invalid("the file is closed"); + return size_; + } + + /* The one call that actually fetches: a ranged GET into the caller's + * memory, so nothing is copied through a buffer of the SDK's own. */ + arrow::Result ReadAt(int64_t position, int64_t nbytes, + void *out) override + { + if (closed_) + return arrow::Status::Invalid("the file is closed"); + if (position < 0 || nbytes < 0) + return arrow::Status::Invalid("read position and length must not " + "be negative"); + + nbytes = std::min(nbytes, std::max(0, size_ - position)); + if (nbytes == 0) + return 0; + + Aws::S3::Model::GetObjectRequest request; + char range[64]; + + snprintf(range, sizeof(range), "bytes=%lld-%lld", + (long long) position, (long long) (position + nbytes - 1)); + request.SetBucket(bucket_.c_str()); + request.SetKey(key_.c_str()); + request.SetRange(range); - if (file_out != NULL) - *file_out = NULL; + /* + * Hand the SDK the destination instead of taking its stringstream: + * the response body is written straight into memory the caller (and + * so the query's memory accounting) already owns. The stream buffer + * outlives the outcome because it is declared before it. + */ + Aws::Utils::Stream::PreallocatedStreamBuf stream_buf( + reinterpret_cast(out), (uint64_t) nbytes); - return DL_ERR_NOT_SUPPORTED; + request.SetResponseStreamFactory([&stream_buf]() { + return Aws::New(DL_S3_ALLOC_TAG, &stream_buf); + }); + + auto outcome = client_->GetObject(request); + + if (!outcome.IsSuccess()) + return status_from_aws("read", bucket_, key_, outcome.GetError()); + + /* + * The buffer handed to the SDK is exactly nbytes long, so a service + * that ignored the range and sent more has already been stopped by + * the stream buffer; what must not happen is reporting those bytes as + * read, which would hand the caller a length its memory does not + * cover. + */ + int64_t got = outcome.GetResult().GetContentLength(); + + if (got < 0 || got > nbytes) + return arrow::Status::IOError( + "read s3://", bucket_, "/", key_, " returned ", got, + " bytes for a ", nbytes, " byte range") + .WithDetail(std::make_shared(DL_ERR_IO, + "ShortRead")); + return got; } - DlErrCode - List(const char *prefix, char ***names_out, int *nnames_out) + arrow::Result> ReadAt(int64_t position, + int64_t nbytes) override { - (void) prefix; + ARROW_ASSIGN_OR_RAISE(auto buffer, + arrow::AllocateResizableBuffer(nbytes, pool_)); - if (names_out != NULL) - *names_out = NULL; - if (nnames_out != NULL) - *nnames_out = 0; + ARROW_ASSIGN_OR_RAISE(int64_t read, + ReadAt(position, nbytes, buffer->mutable_data())); + ARROW_RETURN_NOT_OK(buffer->Resize(read, /* shrink_to_fit = */ false)); + return std::shared_ptr(std::move(buffer)); + } + + arrow::Result Read(int64_t nbytes, void *out) override + { + ARROW_ASSIGN_OR_RAISE(int64_t read, ReadAt(position_, nbytes, out)); + position_ += read; + return read; + } - return DL_ERR_NOT_SUPPORTED; + arrow::Result> Read(int64_t nbytes) override + { + ARROW_ASSIGN_OR_RAISE(auto buffer, ReadAt(position_, nbytes)); + position_ += buffer->size(); + return buffer; } + +private: + std::shared_ptr client_; + std::string bucket_; + std::string key_; + int64_t size_; + arrow::MemoryPool *pool_; + int64_t position_ = 0; + bool closed_ = false; }; +/* ---------------------------------------------------------------------- + * Writing + * ---------------------------------------------------------------------- */ + /* - * A handle the facade can hold. + * Buffers up to a part at a time. A write that stays under the part size + * never starts a multipart upload and goes out as a single PutObject on + * close, so the common case of a small file costs one request; a larger one + * starts the upload only when it has a full part to send. * - * Deriving from the C struct rather than embedding it as a first member is what - * makes recovering the handle defined behaviour: a derived-to-base pointer - * conversion and a static_cast back are guaranteed for any class, while the - * first-member trick is only guaranteed for standard-layout types -- which this - * is not, because of the unique_ptr. The C side still sees a plain - * DatalakeFileSystemData, since that is what the base subobject is. + * Nothing exists under the key until Close(), and Abort() removes the one + * thing this stream may have created, the multipart upload. That is what + * lets the facade stop deleting by path: an abandoned write leaves nothing + * to delete, and an object at that key belongs to somebody else. */ -struct S3FileSystemHandle : public DatalakeFileSystemData +class S3OutputStream : public arrow::io::OutputStream { - std::unique_ptr impl; +public: + S3OutputStream(std::shared_ptr client, std::string bucket, + std::string key, arrow::MemoryPool *pool) + : client_(std::move(client)), bucket_(std::move(bucket)), + key_(std::move(key)), pool_(pool) + { + } + + ~S3OutputStream() override + { + /* A live upload is billable, so it does not outlive the stream. */ + if (!upload_id_.empty()) + (void) AbortUpload(); + } + + arrow::Status Init() + { + ARROW_ASSIGN_OR_RAISE(buffer_, + arrow::AllocateResizableBuffer(0, pool_)); + ARROW_RETURN_NOT_OK(buffer_->Reserve(DL_S3_PART_SIZE)); + return arrow::Status::OK(); + } + + arrow::Status Write(const void *data, int64_t nbytes) override + { + if (closed_) + return arrow::Status::Invalid("the stream is closed"); + if (nbytes < 0) + return arrow::Status::Invalid("cannot write a negative length"); + + const uint8_t *from = reinterpret_cast(data); + + while (nbytes > 0) + { + int64_t room = DL_S3_PART_SIZE - buffer_->size(); + int64_t take = std::min(room, nbytes); + int64_t filled = buffer_->size(); + + ARROW_RETURN_NOT_OK(buffer_->Resize(filled + take, false)); + memcpy(buffer_->mutable_data() + filled, from, (size_t) take); + from += take; + nbytes -= take; + position_ += take; + + if (buffer_->size() == DL_S3_PART_SIZE) + ARROW_RETURN_NOT_OK(UploadPart()); + } + return arrow::Status::OK(); + } + + arrow::Status Flush() override { return arrow::Status::OK(); } + + arrow::Result Tell() const override { return position_; } + + bool closed() const override { return closed_; } + + /* + * Closing is not the same as being done with the upload. A tail part or + * a completion can fail, and the upload is still out there afterwards -- + * so what decides whether there is anything to clean up is whether an + * upload id is still live, never whether Close was called. A failure + * here abandons the upload and reports the original error. + */ + arrow::Status Close() override + { + if (closed_) + return arrow::Status::OK(); + closed_ = true; + + if (upload_id_.empty()) + return PutWholeObject(); + + arrow::Status status; + + if (buffer_->size() > 0) + status = UploadPart(); + if (status.ok()) + status = CompleteUpload(); + if (!status.ok() && !upload_id_.empty()) + (void) AbortUpload(); + return status; + } + + arrow::Status Abort() override + { + closed_ = true; + if (upload_id_.empty()) + return arrow::Status::OK(); /* nothing was ever created */ + return AbortUpload(); + } + +private: + arrow::Status StartUpload() + { + Aws::S3::Model::CreateMultipartUploadRequest request; + + request.SetBucket(bucket_.c_str()); + request.SetKey(key_.c_str()); + + auto outcome = client_->CreateMultipartUpload(request); + + if (!outcome.IsSuccess()) + return status_from_aws("start upload to", bucket_, key_, + outcome.GetError()); + upload_id_ = outcome.GetResult().GetUploadId().c_str(); + return arrow::Status::OK(); + } + + arrow::Status UploadPart() + { + if (upload_id_.empty()) + ARROW_RETURN_NOT_OK(StartUpload()); + + /* Declared before the request, so it outlives the body stream. */ + Aws::Utils::Stream::PreallocatedStreamBuf stream_buf( + buffer_->mutable_data(), (uint64_t) buffer_->size()); + Aws::S3::Model::UploadPartRequest request; + int part = (int) etags_.size() + 1; + + request.SetBucket(bucket_.c_str()); + request.SetKey(key_.c_str()); + request.SetUploadId(upload_id_.c_str()); + request.SetPartNumber(part); + request.SetContentLength(buffer_->size()); + request.SetBody(Aws::MakeShared(DL_S3_ALLOC_TAG, + &stream_buf)); + + auto outcome = client_->UploadPart(request); + + if (!outcome.IsSuccess()) + return status_from_aws("upload part to", bucket_, key_, + outcome.GetError()); + etags_.push_back(outcome.GetResult().GetETag().c_str()); + return buffer_->Resize(0, false); + } + + arrow::Status PutWholeObject() + { + /* Declared before the request, so it outlives the body stream. */ + Aws::Utils::Stream::PreallocatedStreamBuf stream_buf( + buffer_->mutable_data(), (uint64_t) buffer_->size()); + Aws::S3::Model::PutObjectRequest request; + + request.SetBucket(bucket_.c_str()); + request.SetKey(key_.c_str()); + request.SetContentLength(buffer_->size()); + request.SetBody(Aws::MakeShared(DL_S3_ALLOC_TAG, + &stream_buf)); + + auto outcome = client_->PutObject(request); + + if (!outcome.IsSuccess()) + return status_from_aws("write", bucket_, key_, outcome.GetError()); + return arrow::Status::OK(); + } + + arrow::Status CompleteUpload() + { + Aws::S3::Model::CompletedMultipartUpload completed; + Aws::S3::Model::CompleteMultipartUploadRequest request; + + for (size_t i = 0; i < etags_.size(); i++) + { + Aws::S3::Model::CompletedPart part; + + part.SetPartNumber((int) i + 1); + part.SetETag(etags_[i].c_str()); + completed.AddParts(part); + } + + request.SetBucket(bucket_.c_str()); + request.SetKey(key_.c_str()); + request.SetUploadId(upload_id_.c_str()); + request.SetMultipartUpload(completed); + + auto outcome = client_->CompleteMultipartUpload(request); + + if (!outcome.IsSuccess()) + { + arrow::Status status = status_from_aws("finish writing", bucket_, + key_, outcome.GetError()); + + /* The parts are billable until they are abandoned explicitly. */ + (void) AbortUpload(); + return status; + } + upload_id_.clear(); + return arrow::Status::OK(); + } + + arrow::Status AbortUpload() + { + Aws::S3::Model::AbortMultipartUploadRequest request; + + request.SetBucket(bucket_.c_str()); + request.SetKey(key_.c_str()); + request.SetUploadId(upload_id_.c_str()); + + auto outcome = client_->AbortMultipartUpload(request); + + upload_id_.clear(); + if (!outcome.IsSuccess()) + return status_from_aws("abandon the upload to", bucket_, key_, + outcome.GetError()); + return arrow::Status::OK(); + } + + std::shared_ptr client_; + std::string bucket_; + std::string key_; + arrow::MemoryPool *pool_; + std::shared_ptr buffer_; + std::string upload_id_; + std::vector etags_; + int64_t position_ = 0; + bool closed_ = false; }; -static DlErrCode -s3_fs_open(const DatalakeLocation *location, const DlKeyValue *credentials, - int ncredentials, DatalakeFileSystem *fs_out) +/* ---------------------------------------------------------------------- + * The file system + * ---------------------------------------------------------------------- */ + +class S3FileSystem : public arrow::fs::FileSystem { - DlErrCode rc = DL_ERR_INTERNAL; +public: + S3FileSystem(std::shared_ptr client, + const arrow::io::IOContext &io_context) + : arrow::fs::FileSystem(io_context), client_(std::move(client)) + { + } - DL_ABI_GUARD_BEGIN + std::string type_name() const override { return "dl_s3"; } + + bool Equals(const arrow::fs::FileSystem &other) const override { - if (fs_out == NULL) - rc = DL_ERR_INVALID_OPTION; - else + return this == &other; + } + + arrow::Result GetFileInfo(const std::string &path) override + { + std::string bucket, + key; + + split_path(path, &bucket, &key); + if (key.empty()) { /* - * Owned by unique_ptr until the handle is published, so that an - * exception from the second allocation or from Initialize() -- - * which the guard below turns into an error code -- cannot leave - * the first allocation behind. + * The bucket itself. Ask before answering: reporting a bucket + * that is not there as a directory would turn a typo into an + * empty scan instead of an error. */ - std::unique_ptr handle(new S3FileSystemHandle()); + Aws::S3::Model::ListObjectsV2Request probe; + + probe.SetBucket(bucket.c_str()); + probe.SetMaxKeys(1); - *fs_out = NULL; - handle->impl.reset(new S3FileSystem()); - rc = handle->impl->Initialize(location, credentials, ncredentials); + auto outcome = client_->ListObjectsV2(probe); + arrow::fs::FileInfo info(path); - if (rc == DL_OK) - *fs_out = handle.release(); + if (!outcome.IsSuccess()) + { + if (!is_not_found(outcome.GetError())) + return status_from_aws("inspect", bucket, key, + outcome.GetError()); + info.set_type(arrow::fs::FileType::NotFound); + return info; + } + info.set_type(arrow::fs::FileType::Directory); + return info; } + + Aws::S3::Model::HeadObjectRequest request; + + request.SetBucket(bucket.c_str()); + request.SetKey(key.c_str()); + + auto outcome = client_->HeadObject(request); + + if (outcome.IsSuccess()) + { + arrow::fs::FileInfo info(path); + + info.set_type(arrow::fs::FileType::File); + info.set_size(outcome.GetResult().GetContentLength()); + info.set_mtime(std::chrono::system_clock::time_point( + std::chrono::milliseconds( + outcome.GetResult().GetLastModified().Millis()))); + return info; + } + if (!is_not_found(outcome.GetError())) + return status_from_aws("inspect", bucket, key, outcome.GetError()); + + /* No object by that name; it may still be a prefix with objects. */ + ARROW_ASSIGN_OR_RAISE(bool is_prefix, PrefixExists(bucket, key)); + + arrow::fs::FileInfo info(path); + + info.set_type(is_prefix ? arrow::fs::FileType::Directory : + arrow::fs::FileType::NotFound); + return info; } - DL_ABI_GUARD_END(rc, "s3_fs_open"); - return rc; -} + arrow::Result GetFileInfo( + const arrow::fs::FileSelector &select) override + { + std::string bucket, + key; + arrow::fs::FileInfoVector infos; + Aws::String token; + bool more = true; -static void -s3_fs_close(DatalakeFileSystem fs) -{ - DL_CLEANUP_GUARD_BEGIN + split_path(select.base_dir, &bucket, &key); + + const std::string prefix = key.empty() ? std::string() : key + "/"; + + while (more) + { + Aws::S3::Model::ListObjectsV2Request request; + + request.SetBucket(bucket.c_str()); + if (!prefix.empty()) + request.SetPrefix(prefix.c_str()); + if (!select.recursive) + request.SetDelimiter("/"); + if (!token.empty()) + request.SetContinuationToken(token); + + auto outcome = client_->ListObjectsV2(request); + + if (!outcome.IsSuccess()) + return status_from_aws("list", bucket, key, outcome.GetError()); + + const auto &result = outcome.GetResult(); + + for (const auto &object : result.GetContents()) + { + std::string object_key(object.GetKey().c_str()); + + /* The prefix marker some tools write for an empty folder. */ + if (!object_key.empty() && object_key.back() == '/') + continue; + + arrow::fs::FileInfo info(bucket + "/" + object_key); + + info.set_type(arrow::fs::FileType::File); + info.set_size(object.GetSize()); + info.set_mtime(std::chrono::system_clock::time_point( + std::chrono::milliseconds(object.GetLastModified().Millis()))); + infos.push_back(std::move(info)); + } + + for (const auto &common : result.GetCommonPrefixes()) + { + std::string dir(common.GetPrefix().c_str()); + + while (!dir.empty() && dir.back() == '/') + dir.pop_back(); + + arrow::fs::FileInfo info(bucket + "/" + dir); + + info.set_type(arrow::fs::FileType::Directory); + infos.push_back(std::move(info)); + } + + /* + * A service that says "more" without moving the token would spin + * here forever, and this loop is inside a C++ frame where an + * interrupt cannot be checked. + */ + Aws::String next = result.GetNextContinuationToken(); + + more = result.GetIsTruncated(); + if (more && (next.empty() || next == token)) + return arrow::Status::IOError( + "listing s3://", bucket, "/", key, + " did not advance past a truncated page") + .WithDetail(std::make_shared(DL_ERR_IO, + "ListStalled")); + token = next; + } + + /* + * What an empty answer means is the same question for every backend, + * so the facade decides it rather than each of us. + */ + return infos; + } + + arrow::Result> OpenInputStream( + const std::string &path) override { - /* The facade has already cleared its caller's handle. */ - delete static_cast(fs); + ARROW_ASSIGN_OR_RAISE(auto file, OpenInputFile(path)); + return file; } - DL_CLEANUP_GUARD_END; -} -static DlErrCode -s3_fs_list(DatalakeFileSystem fs, const char *prefix, char ***names_out, - int *nnames_out) -{ - DlErrCode rc = DL_ERR_INTERNAL; + arrow::Result> OpenInputFile( + const std::string &path) override + { + std::string bucket, + key; + + split_path(path, &bucket, &key); + if (key.empty()) + return arrow::Status::IOError("s3://", bucket, + " names a bucket, not an object"); - DL_ABI_GUARD_BEGIN + Aws::S3::Model::HeadObjectRequest request; + + request.SetBucket(bucket.c_str()); + request.SetKey(key.c_str()); + + auto outcome = client_->HeadObject(request); + + if (!outcome.IsSuccess()) + return status_from_aws("open", bucket, key, outcome.GetError()); + + return std::make_shared(client_, bucket, key, + outcome.GetResult().GetContentLength(), + io_context().pool()); + } + + arrow::Result> OpenOutputStream( + const std::string &path, + const std::shared_ptr &metadata) override { - S3FileSystemHandle *handle = static_cast(fs); + std::string bucket, + key; + + (void) metadata; + split_path(path, &bucket, &key); + if (key.empty()) + return arrow::Status::IOError("s3://", bucket, + " names a bucket, not an object"); + + auto stream = std::make_shared( + client_, bucket, key, io_context().pool()); - if (handle == NULL) - rc = DL_ERR_INVALID_OPTION; - else - rc = handle->impl->List(prefix, names_out, nnames_out); + ARROW_RETURN_NOT_OK(stream->Init()); + return stream; } - DL_ABI_GUARD_END(rc, "s3_fs_list"); - return rc; -} + arrow::Status DeleteFile(const std::string &path) override + { + std::string bucket, + key; -static DlErrCode -s3_file_open(DatalakeFileSystem fs, const char *path, DatalakeFileMode mode, - DatalakeFile *file_out) -{ - DlErrCode rc = DL_ERR_INTERNAL; + split_path(path, &bucket, &key); + if (key.empty()) + return arrow::Status::IOError("s3://", bucket, + " names a bucket, not an object"); + + Aws::S3::Model::HeadObjectRequest head; + + head.SetBucket(bucket.c_str()); + head.SetKey(key.c_str()); + + auto found = client_->HeadObject(head); + + if (!found.IsSuccess()) + return status_from_aws("delete", bucket, key, found.GetError()); + + Aws::S3::Model::DeleteObjectRequest request; - DL_ABI_GUARD_BEGIN + request.SetBucket(bucket.c_str()); + request.SetKey(key.c_str()); + + auto outcome = client_->DeleteObject(request); + + if (!outcome.IsSuccess()) + return status_from_aws("delete", bucket, key, outcome.GetError()); + return arrow::Status::OK(); + } + + /* + * The rest of the interface is not part of what this module asks of a + * backend (see storage_backend.h), and object storage has no directories + * to create or rename anyway. + */ + arrow::Status CreateDir(const std::string &path, bool recursive) override + { + (void) path; + (void) recursive; + return arrow::Status::NotImplemented("s3: creating a directory"); + } + + arrow::Status DeleteDir(const std::string &path) override { - S3FileSystemHandle *handle = static_cast(fs); + (void) path; + return arrow::Status::NotImplemented("s3: deleting a directory"); + } - if (handle == NULL) - rc = DL_ERR_INVALID_OPTION; - else - rc = handle->impl->OpenFile(path, mode, file_out); + arrow::Status DeleteDirContents(const std::string &path, + bool missing_dir_ok) override + { + (void) path; + (void) missing_dir_ok; + return arrow::Status::NotImplemented("s3: deleting a directory"); } - DL_ABI_GUARD_END(rc, "s3_file_open"); - return rc; -} + arrow::Status DeleteRootDirContents() override + { + return arrow::Status::NotImplemented("s3: deleting a directory"); + } -static DlErrCode -s3_file_read(DatalakeFile file, void *buffer, int64_t length, int64_t *nread) -{ - DlErrCode rc = DL_ERR_INTERNAL; + arrow::Status Move(const std::string &src, const std::string &dest) override + { + (void) src; + (void) dest; + return arrow::Status::NotImplemented("s3: moving an object"); + } + + arrow::Status CopyFile(const std::string &src, const std::string &dest) override + { + (void) src; + (void) dest; + return arrow::Status::NotImplemented("s3: copying an object"); + } + + arrow::Result> OpenAppendStream( + const std::string &path, + const std::shared_ptr &metadata) override + { + (void) path; + (void) metadata; + return arrow::Status::NotImplemented("s3: appending to an object"); + } - DL_ABI_GUARD_BEGIN +private: + arrow::Result PrefixExists(const std::string &bucket, + const std::string &key) { - (void) file; - (void) buffer; - (void) length; + Aws::S3::Model::ListObjectsV2Request request; + + /* + * Without the trailing slash this would also match a sibling whose + * name merely starts with the same characters; with it, an object + * written as the folder marker itself still counts, which is what + * tools that create empty folders leave behind. + */ + request.SetBucket(bucket.c_str()); + request.SetPrefix((key + "/").c_str()); + request.SetMaxKeys(1); + + auto outcome = client_->ListObjectsV2(request); + + if (!outcome.IsSuccess()) + return status_from_aws("inspect", bucket, key, outcome.GetError()); + return outcome.GetResult().GetKeyCount() > 0; + } + + std::shared_ptr client_; +}; - if (nread != NULL) - *nread = 0; +/* ---------------------------------------------------------------------- + * Mounting + * ---------------------------------------------------------------------- */ - rc = DL_ERR_NOT_SUPPORTED; +const char * +option_value(const DlKeyValue *kv, int nkv, const char *key) +{ + for (int i = 0; i < nkv; i++) + { + if (kv[i].key != NULL && strcmp(kv[i].key, key) == 0 && + kv[i].value != NULL && kv[i].value[0] != '\0') + return kv[i].value; } - DL_ABI_GUARD_END(rc, "s3_file_read"); + return NULL; +} + +bool +option_is_true(const char *value) +{ + return value != NULL && + (strcasecmp(value, "true") == 0 || strcasecmp(value, "on") == 0 || + strcasecmp(value, "yes") == 0 || strcasecmp(value, "t") == 0 || + strcasecmp(value, "y") == 0 || strcmp(value, "1") == 0); +} + +arrow::Status +initialize_s3(void) +{ + /* + * The SDK starts threads and opens handles, neither of which survives a + * fork, so this may only ever run in a backend. The facade calls it + * before the first mount in each process and registers the matching + * shutdown there. + */ + Assert(MyProcPid != PostmasterPid); + Aws::InitAPI(sdk_options); + return arrow::Status::OK(); +} - return rc; +void +finalize_s3(void) +{ + Aws::ShutdownAPI(sdk_options); } -static DlErrCode -s3_file_write(DatalakeFile file, const void *buffer, int64_t length) +arrow::Result +mount_s3(const DatalakeLocation *location, const DlKeyValue *kv, int nkv, + const DatalakeStorageHost *host) { - DlErrCode rc = DL_ERR_INTERNAL; + if (location == NULL || location->authority == NULL || + location->authority[0] == '\0') + return arrow::Status::Invalid("s3 location has no bucket"); + + const char *endpoint = option_value(kv, nkv, "endpoint"); + const char *region = option_value(kv, nkv, "region"); + const char *path_style = option_value(kv, nkv, "path_style_access"); + /* + * Named as the DDL names them: these arrive as the options of a user + * mapping, and a backend reading them under some other spelling would be + * a second vocabulary for one thing. + */ + const char *access_key = option_value(kv, nkv, "access_key_id"); + const char *secret_key = option_value(kv, nkv, "secret_access_key"); + const char *session_token = option_value(kv, nkv, "session_token"); + + if (endpoint == NULL) + endpoint = location->endpoint; + if (region == NULL) + region = location->region; + + Aws::S3::S3ClientConfiguration config; - DL_ABI_GUARD_BEGIN + config.region = region != NULL ? region : "us-east-1"; + + /* + * Bounded rather than left to the SDK's defaults: a backend blocked on a + * socket is a session that cannot be cancelled, and an unreachable + * endpoint has to become an error while someone is still waiting for it. + */ + config.connectTimeoutMs = 5000; + config.requestTimeoutMs = 300000; + config.retryStrategy = std::make_shared(3); + + /* + * Virtual-host addressing asks DNS for bucket.host, which is right for + * AWS and wrong for most things you can run yourself. So the server's + * setting decides, and where there is no setting, an explicit endpoint + * means path style and its absence means AWS. + */ + if (path_style != NULL) + config.useVirtualAddressing = !option_is_true(path_style); + else + config.useVirtualAddressing = endpoint == NULL || endpoint[0] == '\0'; + + if (endpoint != NULL && endpoint[0] != '\0') { - (void) file; - (void) buffer; - (void) length; + std::string url(endpoint); - rc = DL_ERR_NOT_SUPPORTED; + if (url.compare(0, 7, "http://") == 0) + { + config.scheme = Aws::Http::Scheme::HTTP; + url = url.substr(7); + } + else if (url.compare(0, 8, "https://") == 0) + { + config.scheme = Aws::Http::Scheme::HTTPS; + url = url.substr(8); + } + while (!url.empty() && url.back() == '/') + url.pop_back(); + config.endpointOverride = url.c_str(); } - DL_ABI_GUARD_END(rc, "s3_file_write"); - return rc; -} + /* + * Falling back to the host's own credentials because half a pair was + * given would run the query as whoever the host is, which is not what + * the user who wrote that mapping asked for. + */ + if ((access_key == NULL) != (secret_key == NULL)) + return arrow::Status::Invalid( + "the user mapping has ", access_key != NULL ? + "access_key_id but no secret_access_key" : + "secret_access_key but no access_key_id"); + if (access_key == NULL && session_token != NULL) + return arrow::Status::Invalid( + "the user mapping has session_token but no access_key_id"); -static DlErrCode -s3_file_close(DatalakeFile file) -{ - DlErrCode rc = DL_ERR_INTERNAL; + std::shared_ptr client; + + if (access_key != NULL && secret_key != NULL) + { + /* Credentials the user gave us, through a user mapping. */ + auto provider = + Aws::MakeShared( + DL_S3_ALLOC_TAG, access_key, secret_key, + session_token != NULL ? session_token : ""); - DL_ABI_GUARD_BEGIN + client = std::make_shared(provider, nullptr, config); + } + else { - (void) file; + /* None given: whatever the environment already grants this host. */ + auto provider = + Aws::MakeShared( + DL_S3_ALLOC_TAG); - rc = DL_ERR_NOT_SUPPORTED; + client = std::make_shared(provider, nullptr, config); } - DL_ABI_GUARD_END(rc, "s3_file_close"); - return rc; + DatalakeMountedFs mounted; + + mounted.fs = std::make_shared( + client, arrow::io::IOContext(dl_storage_host_pool(host))); + mounted.root = std::string(location->authority) + + (location->path_prefix != NULL ? location->path_prefix : ""); + return mounted; } -static void -s3_file_abort(DatalakeFile file) +} /* namespace */ + +static const DatalakeStorageBackend s3_storage_backend = { + DL_STORAGE_ABI_VERSION, + sizeof(DatalakeStorageBackend), + "s3", + ARROW_VERSION_STRING, + DL_STORAGE_ABI_FINGERPRINT, + mount_s3, + initialize_s3, + finalize_s3 +}; + +#else /* !DL_HAVE_AWS_SDK */ + +static arrow::Result +mount_s3(const DatalakeLocation *, const DlKeyValue *, int, + const DatalakeStorageHost *) { - DL_CLEANUP_GUARD_BEGIN - { - (void) file; - } - DL_CLEANUP_GUARD_END; + return arrow::Status::NotImplemented( + "datalake_fdw was built without the AWS SDK for C++, so s3:// " + "locations cannot be opened; rebuild the extension with " + "AWS_SDK_PREFIX= pointing at an installed SDK"); } -static const struct DatalakeStorageOps s3_storage_ops = { - s3_fs_open, - s3_fs_close, - s3_fs_list, - s3_file_open, - s3_file_read, - s3_file_write, - s3_file_close, - s3_file_abort +static const DatalakeStorageBackend s3_storage_backend = { + DL_STORAGE_ABI_VERSION, + sizeof(DatalakeStorageBackend), + "s3", + ARROW_VERSION_STRING, + DL_STORAGE_ABI_FINGERPRINT, + mount_s3, + NULL, + NULL }; +#endif /* DL_HAVE_AWS_SDK */ + DlErrCode datalake_register_s3_backend(void) { - return datalake_register_storage_backend("s3", &s3_storage_ops); + return datalake_register_storage_backend(&s3_storage_backend); } diff --git a/contrib/datalake_fdw/src/common/storage_arrow.h b/contrib/datalake_fdw/src/common/storage_arrow.h new file mode 100644 index 00000000000..76095f07a1a --- /dev/null +++ b/contrib/datalake_fdw/src/common/storage_arrow.h @@ -0,0 +1,93 @@ +/*------------------------------------------------------------------------- + * + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * + * storage_arrow.h + * C++ access to the Arrow objects behind the storage facade. + * + * IDENTIFICATION + * contrib/datalake_fdw/src/common/storage_arrow.h + * + *------------------------------------------------------------------------- + */ + +#ifndef STORAGE_ARROW_H +#define STORAGE_ARROW_H + +#include +#include + +#include +#include +#include +#include + +#include "common/file_system_wrapper.h" + +arrow::fs::FileSystem *dl_storage_arrow_fs(DatalakeFileSystem fs); + +/* + * Open a file below a mount, for the format layer, under the same rules the C + * facade applies: a read must find the file, and a write must not. Both take + * a path relative to the mount root, and both refuse one that could climb out + * of it. They exist so that "creating a file never replaces one" is decided + * once, rather than once per format. + */ +arrow::Result> + dl_storage_open_input(DatalakeFileSystem fs, const char *relative); +arrow::Result> + dl_storage_open_output(DatalakeFileSystem fs, const char *relative); +std::string dl_storage_native_path(DatalakeFileSystem fs, + const char *relative); + +class DlStatusDetail : public arrow::StatusDetail +{ +public: + DlStatusDetail(DlErrCode code, std::string type); + const char *type_id() const override; + std::string ToString() const override; + DlErrCode code() const; + const std::string &type() const; + +private: + DlErrCode code_; + std::string type_; +}; + +/* + * Hand the mount's credential values to the error layer, which removes them + * from everything recorded afterwards. Called once per mount; nothing below + * has to carry them around. + */ +void dl_storage_remember_secrets(const DlKeyValue *kv, int nkv); + +/* Turn an Arrow status into a DlErrCode and record it. */ +DlErrCode dl_storage_status_to_err(const arrow::Status &status, + const char *operation); + +/* + * Whether a path may be joined onto a mount root: relative, and free of "." + * and ".." components. The root names where a volume lives, it does not + * confine what a path can reach, so the check belongs before the join. + */ +bool dl_storage_path_is_safe(const char *relative); + +/* The same status with its DlErrCode attached, for a caller that hands it on. */ +arrow::Status dl_storage_classify(arrow::Status status); + +#endif /* STORAGE_ARROW_H */ diff --git a/contrib/datalake_fdw/src/common/storage_backend.h b/contrib/datalake_fdw/src/common/storage_backend.h new file mode 100644 index 00000000000..a7717a19acf --- /dev/null +++ b/contrib/datalake_fdw/src/common/storage_backend.h @@ -0,0 +1,144 @@ +/*------------------------------------------------------------------------- + * + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * + * storage_backend.h + * Public contract for pluggable storage backends. + * + * IDENTIFICATION + * contrib/datalake_fdw/src/common/storage_backend.h + * + *------------------------------------------------------------------------- + */ + +#ifndef STORAGE_BACKEND_H +#define STORAGE_BACKEND_H + +#include +#include + +#include +#include + +#include +#include +#include +#include +#include + +#include "datalake_location.h" +#include "dl_err.h" +#include "dl_kv.h" + +#define DL_STORAGE_ABI_VERSION 1 + +#define DL_STORAGE_STRINGIFY_DETAIL(value) #value +#define DL_STORAGE_STRINGIFY(value) DL_STORAGE_STRINGIFY_DETAIL(value) + +#if defined(__clang__) +#define DL_STORAGE_COMPILER_FINGERPRINT \ + "clang" DL_STORAGE_STRINGIFY(__clang_major__) +#elif defined(__GNUC__) +#define DL_STORAGE_COMPILER_FINGERPRINT \ + "gcc" DL_STORAGE_STRINGIFY(__GNUC__) +#else +#define DL_STORAGE_COMPILER_FINGERPRINT "unknown" +#endif + +#ifdef _GLIBCXX_USE_CXX11_ABI +#define DL_STORAGE_CXX11_ABI_FINGERPRINT \ + DL_STORAGE_STRINGIFY(_GLIBCXX_USE_CXX11_ABI) +#else +#define DL_STORAGE_CXX11_ABI_FINGERPRINT "na" +#endif + +#define DL_STORAGE_ABI_FINGERPRINT \ + DL_STORAGE_COMPILER_FINGERPRINT ";cxx11abi=" \ + DL_STORAGE_CXX11_ABI_FINGERPRINT ";arrow=" ARROW_VERSION_STRING + +struct DatalakeMountedFs +{ + std::shared_ptr fs; + std::string root; +}; + +/* + * Services supplied by the host. Future versions may append fields, so a + * backend reads a field only after struct_size says it is there -- which is + * what dl_storage_host_pool() below does for the one field there is today. + */ +struct DatalakeStorageHost +{ + uint32_t struct_size; + arrow::MemoryPool *pool; /* memory a backend allocates through is + * charged to the query, so IOContext and + * every Buffer must come from here */ +}; + +/* + * The pool to allocate through, or Arrow's default when the host predates the + * field. Allocating outside the host's pool means the memory escapes + * Cloudberry's accounting, so a backend should always route Arrow through it. + */ +static inline arrow::MemoryPool * +dl_storage_host_pool(const DatalakeStorageHost *host) +{ + size_t needed = offsetof(DatalakeStorageHost, pool) + + sizeof(((DatalakeStorageHost *) nullptr)->pool); + + if (host == nullptr || host->struct_size < needed || host->pool == nullptr) + return arrow::default_memory_pool(); + return host->pool; +} + +/* + * A backend only constructs an Arrow filesystem. The facade uses precisely + * GetFileInfo(path), GetFileInfo(FileSelector), OpenInputFile and its + * GetSize/ReadAt/Read methods, OpenOutputStream and its Write/Close/Abort + * methods, and DeleteFile. Other methods may return NotImplemented. + * + * Instances are borrowed by the registry and therefore need static lifetime. + * struct_size is prefix-compatible: future versions may append fields. + */ +struct DatalakeStorageBackend +{ + uint32_t abi_version; + uint32_t struct_size; + const char *uri_scheme; + const char *arrow_version; + const char *abi_fingerprint; + arrow::Result (*mount) (const DatalakeLocation *, + const DlKeyValue *kv, int nkv, + const DatalakeStorageHost *host); + arrow::Status (*initialize) (void); + void (*finalize) (void); +}; + +#ifdef __cplusplus +extern "C" +{ +#endif + +extern DlErrCode datalake_register_storage_backend( + const DatalakeStorageBackend *backend); + +#ifdef __cplusplus +} +#endif + +#endif /* STORAGE_BACKEND_H */ diff --git a/contrib/datalake_fdw/src/common/storage_backend_register.h b/contrib/datalake_fdw/src/common/storage_backend_register.h new file mode 100644 index 00000000000..eacb4e98912 --- /dev/null +++ b/contrib/datalake_fdw/src/common/storage_backend_register.h @@ -0,0 +1,81 @@ +/*------------------------------------------------------------------------- + * + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * + * storage_backend_register.h + * Order-independent registration helper for storage plugins. + * + * IDENTIFICATION + * contrib/datalake_fdw/src/common/storage_backend_register.h + * + *------------------------------------------------------------------------- + */ + +#ifndef STORAGE_BACKEND_REGISTER_H +#define STORAGE_BACKEND_REGISTER_H + +/* Arrow headers must precede PostgreSQL headers in a C++ translation unit. */ +#include "storage_backend.h" + +extern "C" +{ +#include "postgres.h" + +#include "fmgr.h" +#include "utils/elog.h" +} + +/* + * Register a backend, whatever order the libraries were loaded in. + * + * Loading the extension is PostgreSQL's job, and PostgreSQL reports a missing + * library or symbol by raising an error, which unwinds with longjmp -- through + * this plugin's C++ frames, skipping their destructors. So the load happens + * inside PG_TRY and comes back as a value instead. + */ +static inline DlErrCode +datalake_storage_register(const DatalakeStorageBackend *backend) +{ + typedef DlErrCode (*dl_register_fn) (const DatalakeStorageBackend *); + volatile DlErrCode rc = DL_ERR_INTERNAL; + + PG_TRY(); + { + dl_register_fn fn = reinterpret_cast( + load_external_function("$libdir/datalake_fdw", + "datalake_register_storage_backend", + true, NULL)); + + rc = fn(backend); + } + PG_CATCH(); + { + /* + * The caller is a plugin's _PG_init, which is entitled to decide for + * itself whether it can carry on; what it must not get is an unwind + * through its own frames. + */ + FlushErrorState(); + rc = DL_ERR_NOT_SUPPORTED; + } + PG_END_TRY(); + + return rc; +} + +#endif /* STORAGE_BACKEND_REGISTER_H */ diff --git a/contrib/datalake_fdw/src/format/arrow_support.cpp b/contrib/datalake_fdw/src/format/arrow_support.cpp index a4e951eeb9f..8ed1622cd6c 100644 --- a/contrib/datalake_fdw/src/format/arrow_support.cpp +++ b/contrib/datalake_fdw/src/format/arrow_support.cpp @@ -39,6 +39,7 @@ #include +#include "common/storage_arrow.h" #include "format/arrow_support.h" extern "C" @@ -56,6 +57,23 @@ DlArrowStatus(const arrow::Status &status, const char *operation) if (status.ok()) return DL_OK; + /* + * A status that came from the storage layer already carries the answer: + * only the backend knows that a 404 from one service and a missing file + * on another are both "not found". Reclassifying it here by status code + * would throw that away. + */ + if (status.detail() != nullptr && + strcmp(status.detail()->type_id(), "datalake::DlStatusDetail") == 0) + { + const DlStatusDetail *detail = + static_cast(status.detail().get()); + + dl_error_set(detail->code(), operation, detail->type().c_str(), + status.message().c_str()); + return detail->code(); + } + switch (status.code()) { case arrow::StatusCode::IOError: @@ -72,13 +90,25 @@ DlArrowStatus(const arrow::Status &status, const char *operation) case arrow::StatusCode::OutOfMemory: code = DL_ERR_OUT_OF_MEMORY; break; + case arrow::StatusCode::AlreadyExists: + code = DL_ERR_ALREADY_EXISTS; + break; default: code = DL_ERR_INTERNAL; break; } - dl_error_set(code, operation, arrow::Status::CodeAsString(status.code()).c_str(), - status.message().c_str()); + /* + * Arrow's own name for a code it does not print is "Unknown", which says + * less than nothing next to a message that already explains itself. + */ + { + std::string type = arrow::Status::CodeAsString(status.code()); + + dl_error_set(code, operation, + type == "Unknown" ? NULL : type.c_str(), + status.message().c_str()); + } return code; } diff --git a/contrib/datalake_fdw/src/format/format.h b/contrib/datalake_fdw/src/format/format.h index 436240aa5f5..d9d052e9259 100644 --- a/contrib/datalake_fdw/src/format/format.h +++ b/contrib/datalake_fdw/src/format/format.h @@ -32,6 +32,8 @@ #include #include +#include "common/file_system_wrapper.h" + #include "common/dl_err.h" /* Arrow C data interface: stable public ABI. */ @@ -72,7 +74,8 @@ struct ArrowArray { */ typedef struct Fragment { - const char *path; + DatalakeFileSystem fs; /* where the file lives; never NULL */ + const char *path; /* relative to that file system's root */ int first_row_group; /* 0-based */ int n_row_groups; /* 0 == to the end of the file */ } Fragment; @@ -170,15 +173,18 @@ struct FormatWriter { const FormatWriterOps *ops; void *impl; }; /* * Bumped when an existing field changes meaning; appending does not need it. * 2: ProjectionSet names field ids rather than positions in the file. + * 3: a fragment and a writer name a file system, and their paths are relative + * to its root, so a format reads and writes wherever the volume is. */ -#define DL_FORMAT_ABI_VERSION 2 +#define DL_FORMAT_ABI_VERSION 3 typedef struct FormatRoutine { uint32_t abi_version, struct_size; /* same prefix-compat semantics as meta engine */ const char *name; /* "parquet" */ DlErrCode (*open_reader)(const Fragment *, const ProjectionSet *, const RowGroupFilterSet *, FormatReader **out); - DlErrCode (*open_writer)(const char *path, /* TupleDesc */ void *tupdesc, + DlErrCode (*open_writer)(DatalakeFileSystem fs, const char *path, + /* TupleDesc */ void *tupdesc, const WriterOptions *, FormatWriter **out); } FormatRoutine; diff --git a/contrib/datalake_fdw/src/format/parquet/parquet_internal.h b/contrib/datalake_fdw/src/format/parquet/parquet_internal.h index f7acbdd2f94..5b7b6e1129b 100644 --- a/contrib/datalake_fdw/src/format/parquet/parquet_internal.h +++ b/contrib/datalake_fdw/src/format/parquet/parquet_internal.h @@ -40,7 +40,8 @@ extern DlErrCode parquet_open_reader(const Fragment *fragment, const RowGroupFilterSet *filters, FormatReader **out); -extern DlErrCode parquet_open_writer(const char *path, void *tupdesc, +extern DlErrCode parquet_open_writer(DatalakeFileSystem fs, const char *path, + void *tupdesc, const WriterOptions *options, FormatWriter **out); diff --git a/contrib/datalake_fdw/src/format/parquet/parquet_read.cpp b/contrib/datalake_fdw/src/format/parquet/parquet_read.cpp index c5432fd2292..72e06bb253f 100644 --- a/contrib/datalake_fdw/src/format/parquet/parquet_read.cpp +++ b/contrib/datalake_fdw/src/format/parquet/parquet_read.cpp @@ -39,6 +39,7 @@ #include #include +#include "common/storage_arrow.h" #include "format/arrow_support.h" #include "am_iceberg/pg_iceberg_guc.h" @@ -297,7 +298,7 @@ parquet_open_reader(const Fragment *fragment, const ProjectionSet *projection, return DL_ARG_ERROR("open_reader"); *out = NULL; - if (fragment == NULL || fragment->path == NULL) + if (fragment == NULL || fragment->path == NULL || fragment->fs == NULL) return DL_ARG_ERROR("open_reader"); /* @@ -325,8 +326,8 @@ parquet_open_reader(const Fragment *fragment, const ProjectionSet *projection, parquet::arrow::FileReaderBuilder builder; parquet::ArrowReaderProperties properties; - arrow::Result> file = - arrow::io::ReadableFile::Open(fragment->path, pool); + arrow::Result> file = + dl_storage_open_input(fragment->fs, fragment->path); if (!file.ok()) return DlArrowStatus(file.status(), "open a Parquet file"); diff --git a/contrib/datalake_fdw/src/format/parquet/parquet_write.cpp b/contrib/datalake_fdw/src/format/parquet/parquet_write.cpp index ea60301df97..d9268f93909 100644 --- a/contrib/datalake_fdw/src/format/parquet/parquet_write.cpp +++ b/contrib/datalake_fdw/src/format/parquet/parquet_write.cpp @@ -45,6 +45,7 @@ #include #include +#include "common/storage_arrow.h" #include "format/arrow_support.h" extern "C" @@ -60,9 +61,9 @@ extern "C" struct ParquetWriter { FormatWriter base; - std::string path; + std::string path; /* relative to the mount, for messages */ std::shared_ptr schema; - std::shared_ptr sink; + std::shared_ptr sink; std::unique_ptr writer; /* @@ -98,23 +99,22 @@ class ParquetReleaseBatch }; /* - * Gives up on the file being written. The sink is closed first and the writer - * left to its destructor: a Parquet writer writes the footer when it closes, - * and against a sink that is already closed it cannot -- which is what stops a - * complete, valid, truncated file appearing at the path if the unlink does not - * take. What it leaves then has no footer, so nothing can read it, and that is - * why the unlink's result is not worth reporting. + * Gives up on the file being written. Aborting the sink is what removes it: + * a stream cleans up whatever it created and nothing else, which on a local + * file means unlinking the one it created and on object storage means + * abandoning the upload, leaving no object at all. Deleting by path from here + * would instead reach whatever is at that name by now, which after a failed + * write may belong to another writer entirely. * - * The file is ours to delete: parquet_open_writer() created it with O_EXCL, so - * nothing was at the path before, and nothing this removes was anyone else's. + * The writer itself is left to its destructor. A Parquet writer writes the + * footer when it closes, and against an aborted sink it cannot, so what a + * half-written file leaves behind is unreadable rather than plausible. */ static void parquet_discard(ParquetWriter *impl) { if (impl->sink != nullptr) - (void) impl->sink->Close(); - - (void) unlink(impl->path.c_str()); + (void) impl->sink->Abort(); } /* @@ -387,7 +387,7 @@ parquet_compression(const char *name, arrow::Compression::type *out) } DlErrCode -parquet_open_writer(const char *path, void *tupdesc_arg, +parquet_open_writer(DatalakeFileSystem fs, const char *path, void *tupdesc_arg, const WriterOptions *options, FormatWriter **out) { DlErrCode result = DL_OK; @@ -396,7 +396,7 @@ parquet_open_writer(const char *path, void *tupdesc_arg, return DL_ARG_ERROR("open_writer"); *out = NULL; - if (path == NULL || tupdesc_arg == NULL) + if (fs == NULL || path == NULL || tupdesc_arg == NULL) return DL_ARG_ERROR("open_writer"); DL_ABI_GUARD_BEGIN @@ -406,7 +406,6 @@ parquet_open_writer(const char *path, void *tupdesc_arg, parquet::WriterProperties::Builder properties; arrow::Compression::type compression = arrow::Compression::SNAPPY; DlErrCode rc; - int fd; impl->path = path; impl->schema = DlArrowSchemaFromTupleDesc((TupleDesc) tupdesc_arg, @@ -435,44 +434,17 @@ parquet_open_writer(const char *path, void *tupdesc_arg, impl->pending_rows = 0; /* - * Created here rather than by Arrow. Arrow's path form opens with - * O_TRUNC, which would empty a file that was already there -- and this - * writer deletes the file it holds whenever it cannot finish it, so a - * truncated file would then be a deleted one. With O_EXCL the kernel - * answers "did I create this", and the writer only ever deletes what - * it created. A lake's data file names are unique by construction, so - * a path that exists is a mistake, and refusing it is right anyway. + * The storage layer creates it, and refuses if something is already + * there: a lake's data file names are unique by construction, so a + * path that exists is a mistake rather than something to overwrite. + * The stream that comes back owns what it created, which is what lets + * parquet_discard() give the file up without deleting by path. */ - fd = open(path, O_WRONLY | O_CREAT | O_EXCL, pg_file_create_mode); - if (fd < 0) - { - int saved_errno = errno; - std::string message; - - if (saved_errno == EEXIST) - { - message = std::string("\"") + path + "\" already exists"; - dl_error_set(DL_ERR_ALREADY_EXISTS, "create a Parquet file", NULL, - message.c_str()); - return DL_ERR_ALREADY_EXISTS; - } - - message = std::string("could not create \"") + path + "\": " + - strerror(saved_errno); - dl_error_set(DL_ERR_IO, "create a Parquet file", NULL, message.c_str()); - return DL_ERR_IO; - } - - /* From here the file exists and is ours, so every failure discards it. */ - arrow::Result> sink = - arrow::io::FileOutputStream::Open(fd); + arrow::Result> sink = + dl_storage_open_output(fs, path); if (!sink.ok()) - { - (void) close(fd); /* Arrow took nothing */ - parquet_discard(impl.get()); return DlArrowStatus(sink.status(), "create a Parquet file"); - } impl->sink = *sink; /* diff --git a/contrib/datalake_fdw/src/iceberg_volume_fdw/iceberg_volume_fdw.c b/contrib/datalake_fdw/src/iceberg_volume_fdw/iceberg_volume_fdw.c index ab5f5c735d0..6b5c85c3b89 100644 --- a/contrib/datalake_fdw/src/iceberg_volume_fdw/iceberg_volume_fdw.c +++ b/contrib/datalake_fdw/src/iceberg_volume_fdw/iceberg_volume_fdw.c @@ -33,6 +33,9 @@ #include "catalog/pg_foreign_data_wrapper.h" #include "catalog/pg_foreign_server.h" #include "catalog/pg_user_mapping.h" +#include "commands/dbcommands.h" +#include "miscadmin.h" +#include "utils/acl.h" #include "commands/defrem.h" #include "common/dl_option_util.h" #include "fmgr.h" @@ -148,8 +151,94 @@ iceberg_volume_fdw_validator(PG_FUNCTION_ARGS) (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("invalid iceberg volume %s \"%s\"", DATALAKE_ICEBERG_VOLUME_BASE_PATH, - volume_options.base_path), + pg_iceberg_redacted_location_uri(volume_options.base_path)), errdetail("%s", parse_detail))); PG_RETURN_VOID(); } + +/* + * Everything the storage layer needs to reach one volume: where it is, and + * under what credentials. + * + * The options are read the way the validator reads them, so a volume that was + * accepted at CREATE SERVER resolves here too. A user mapping is optional -- + * without one the backend falls back to whatever credentials the host already + * has, which is how an instance profile or a ticket cache is meant to be used + * -- so this returns an empty credential set rather than refusing. + */ +void +iceberg_volume_resolve(const char *server_name, Oid userid, + DatalakeLocation *location_out, + DlKeyValue **kv_out, int *nkv_out) +{ + ForeignServer *server; + IcebergVolumeOptions *options; + MetaKv *credentials; + DlKeyValue *kv; + char *parse_detail = NULL; + AclResult aclresult; + DlErrCode rc; + int ncredentials = 0; + int nkv = 0; + + Assert(location_out != NULL && kv_out != NULL && nkv_out != NULL); + + server = GetForeignServerByName(server_name, false); + + aclresult = object_aclcheck(ForeignServerRelationId, server->serverid, + userid, ACL_USAGE); + if (aclresult != ACLCHECK_OK) + aclcheck_error(aclresult, OBJECT_FOREIGN_SERVER, server->servername); + + options = get_iceberg_volume_options(server); + if (options->foreign_volume.base_path == NULL) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("iceberg volume server option \"%s\" is required", + DATALAKE_ICEBERG_VOLUME_BASE_PATH))); + + rc = pg_iceberg_parse_location(options->foreign_volume.base_path, + options->volume_server.endpoint, + options->volume_server.region, + location_out, &parse_detail); + if (rc != DL_OK) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("invalid iceberg volume %s", + DATALAKE_ICEBERG_VOLUME_BASE_PATH), + errdetail("%s", parse_detail))); + + /* + * The settings the server carries, then the current user's credentials. + * Backends read these by the names they were written under in DDL, so + * nothing in between has to know what any particular protocol wants. + */ + credentials = pg_iceberg_resolve_credentials(server->serverid, userid, + &ncredentials); + kv = (DlKeyValue *) palloc0((3 + ncredentials) * sizeof(DlKeyValue)); + + if (options->volume_server.endpoint != NULL) + { + kv[nkv].key = DATALAKE_ICEBERG_VOLUME_ENDPOINT; + kv[nkv].value = options->volume_server.endpoint; + nkv++; + } + if (options->volume_server.region != NULL) + { + kv[nkv].key = DATALAKE_ICEBERG_VOLUME_REGION; + kv[nkv].value = options->volume_server.region; + nkv++; + } + if (options->volume_server.path_style_access_set) + { + kv[nkv].key = DATALAKE_ICEBERG_VOLUME_PATH_STYLE_ACCESS; + kv[nkv].value = options->volume_server.path_style_access ? "true" : "false"; + nkv++; + } + for (int i = 0; i < ncredentials; i++) + kv[nkv++] = credentials[i]; + + *kv_out = kv; + *nkv_out = nkv; +} diff --git a/contrib/datalake_fdw/src/iceberg_volume_fdw/iceberg_volume_option.h b/contrib/datalake_fdw/src/iceberg_volume_fdw/iceberg_volume_option.h index 2b4ab16b46c..d1554df2c60 100644 --- a/contrib/datalake_fdw/src/iceberg_volume_fdw/iceberg_volume_option.h +++ b/contrib/datalake_fdw/src/iceberg_volume_fdw/iceberg_volume_option.h @@ -37,6 +37,8 @@ #include "postgres.h" #include "common/dl_option_util.h" +#include "common/datalake_location.h" +#include "common/dl_kv.h" #include "foreign/foreign.h" #include "nodes/pg_list.h" @@ -143,4 +145,14 @@ extern void parse_iceberg_foreign_volume_options(IcebergForeignVolumeOptions *op */ extern IcebergVolumeOptions *get_iceberg_volume_options(ForeignServer *server); +/* + * Resolve a volume by name into the location its data lives at and the + * options a storage backend needs to reach it. Reports through ereport: the + * server has to exist and the user has to be allowed to use it. A user + * mapping is optional, so an empty credential set is a valid answer. + */ +extern void iceberg_volume_resolve(const char *server_name, Oid userid, + DatalakeLocation *location_out, + DlKeyValue **kv_out, int *nkv_out); + #endif /* ICEBERG_VOLUME_OPTION_H */ diff --git a/contrib/datalake_fdw/src/test/datalake_fdw_test.c b/contrib/datalake_fdw/src/test/datalake_fdw_test.c index e6c96f0a57a..a4b8954e795 100644 --- a/contrib/datalake_fdw/src/test/datalake_fdw_test.c +++ b/contrib/datalake_fdw/src/test/datalake_fdw_test.c @@ -37,6 +37,8 @@ #include "postgres.h" +#include + #include "catalog/pg_type.h" #include "executor/spi.h" #include "funcapi.h" @@ -46,13 +48,172 @@ #include "utils/tuplestore.h" #include "am_iceberg/pg_iceberg_guc.h" +#include "am_iceberg/pg_iceberg_options.h" +#include "iceberg_volume_fdw/iceberg_volume_option.h" #include "common/dl_err.h" +#include "common/file_system_wrapper.h" #include "format/arrow_builder.h" #include "format/arrow_decode.h" #include "format/format.h" PG_FUNCTION_INFO_V1(datalake_parquet_write); PG_FUNCTION_INFO_V1(datalake_parquet_read); +PG_FUNCTION_INFO_V1(datalake_storage_write_text); +PG_FUNCTION_INFO_V1(datalake_storage_read_text); +PG_FUNCTION_INFO_V1(datalake_storage_list); +PG_FUNCTION_INFO_V1(datalake_storage_delete); +PG_FUNCTION_INFO_V1(datalake_storage_probe); +PG_FUNCTION_INFO_V1(datalake_storage_register_bad); + +extern DlErrCode datalake_test_register_bad_storage_backend(const char *kind); + +static DlKeyValue * +storage_kv(FunctionCallInfo fcinfo, int argno, int *nkv) +{ + Datum *values; + bool *nulls; + DlKeyValue *kv; + int i; + + *nkv = 0; + if (PG_ARGISNULL(argno)) + return NULL; + + deconstruct_array(PG_GETARG_ARRAYTYPE_P(argno), TEXTOID, -1, false, + TYPALIGN_INT, &values, &nulls, nkv); + kv = palloc0(*nkv * sizeof(*kv)); + for (i = 0; i < *nkv; i++) + { + char *equal; + + if (nulls[i]) + ereport(ERROR, + (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED), + errmsg("a storage option cannot be null"))); + kv[i].key = TextDatumGetCString(values[i]); + equal = strchr(kv[i].key, '='); + if (equal == NULL || equal == kv[i].key) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("storage option must have the form key=value"))); + *equal = '\0'; + kv[i].value = equal + 1; + } + return kv; +} + +static void +storage_parse_uri(const char *uri, bool leaf, DatalakeLocation *location, + char **relative) +{ + char *detail = NULL; + DlErrCode rc; + + /* + * The production parser, dltest included: it accepts any scheme a backend + * has registered, so the test backend is addressed exactly the way a + * third party's would be. + */ + rc = pg_iceberg_parse_location(uri, NULL, NULL, location, &detail); + if (rc != DL_OK) + { + dl_error_set(rc, "parse storage location", NULL, detail); + dl_error_report(ERROR, rc, "parse storage location"); + } + + *relative = pstrdup(""); + if (leaf) + { + char *slash = strrchr(location->path_prefix, '/'); + + if (slash == NULL || slash[1] == '\0') + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("storage URI must name a file"))); + *relative = pstrdup(slash + 1); + if (slash == location->path_prefix) + location->path_prefix = pstrdup( + strcmp(location->scheme, "s3") == 0 ? "" : "/"); + else + *slash = '\0'; + } +} + +/* + * Mount whatever the caller named. A bare path is a local absolute path -- + * which is what the Parquet cases have always passed -- and a URI names a + * volume's scheme; when a volume is given, its server options and the calling + * user's credentials are what the backend gets. + */ +static DatalakeFileSystem +storage_open_volume(const char *path_or_uri, const char *volume, bool leaf, + char **relative) +{ + DatalakeLocation location; + DatalakeFileSystem fs = NULL; + DlKeyValue *kv = NULL; + char *uri; + int nkv = 0; + DlErrCode rc; + + uri = strstr(path_or_uri, "://") != NULL ? pstrdup(path_or_uri) : + psprintf("file://%s", path_or_uri); + + storage_parse_uri(uri, leaf, &location, relative); + + if (volume != NULL) + { + DatalakeLocation volume_location; + Size prefix_len; + + iceberg_volume_resolve(volume, GetUserId(), &volume_location, &kv, &nkv); + + /* + * A volume's credentials belong to the volume's storage. Without + * this, naming any volume would lend its keys to any bucket the + * caller cared to type. + */ + prefix_len = strlen(volume_location.path_prefix); + if (strcmp(location.scheme, volume_location.scheme) != 0 || + strcmp(location.authority, volume_location.authority) != 0 || + strncmp(location.path_prefix, volume_location.path_prefix, + prefix_len) != 0 || + (location.path_prefix[prefix_len] != '\0' && + location.path_prefix[prefix_len] != '/')) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("\"%s\" is not inside volume \"%s\"", + path_or_uri, volume))); + + /* The volume says how to reach it; the URI says which object. */ + location.endpoint = volume_location.endpoint; + location.region = volume_location.region; + } + + rc = datalake_fs_open(&location, kv, nkv, &fs); + if (rc != DL_OK) + dl_error_report(ERROR, rc, "open storage"); + return fs; +} + +static DatalakeFileSystem +storage_open_uri(FunctionCallInfo fcinfo, int uri_arg, int kv_arg, bool leaf, + char **relative) +{ + DatalakeLocation location; + DatalakeFileSystem fs = NULL; + DlKeyValue *kv; + char *uri = text_to_cstring(PG_GETARG_TEXT_PP(uri_arg)); + int nkv; + DlErrCode rc; + + storage_parse_uri(uri, leaf, &location, relative); + kv = storage_kv(fcinfo, kv_arg, &nkv); + rc = datalake_fs_open(&location, kv, nkv, &fs); + if (rc != DL_OK) + dl_error_report(ERROR, rc, "open storage"); + return fs; +} /* * The SQL declaration and the C function have to agree on the argument list, @@ -130,16 +291,25 @@ datalake_parquet_write(PG_FUNCTION_ARGS) WriterOptions options = {0}; FormatWriter *volatile open_writer = NULL; DlArrowBuilder volatile open_builder = NULL; + DatalakeFileSystem volatile open_fs = NULL; + char *relative; long batch_rows = iceberg_batch_rows; int64 written = 0; MemoryContext row_context; - check_nargs(fcinfo, 4); + check_nargs(fcinfo, 5); + /* Not STRICT, because volume is optional; the rest are not. */ + if (PG_ARGISNULL(0) || PG_ARGISNULL(1) || PG_ARGISNULL(2) || PG_ARGISNULL(3)) + PG_RETURN_NULL(); path = text_to_cstring(PG_GETARG_TEXT_PP(0)); query = text_to_cstring(PG_GETARG_TEXT_PP(1)); row_group_size = PG_GETARG_INT32(2); compression = text_to_cstring(PG_GETARG_TEXT_PP(3)); routine = parquet_routine(); + open_fs = storage_open_volume(path, + PG_ARGISNULL(4) ? NULL : + text_to_cstring(PG_GETARG_TEXT_PP(4)), + true, &relative); /* * Bounded above as well as below, and by the same number as @@ -209,7 +379,8 @@ datalake_parquet_write(PG_FUNCTION_ARGS) { tupdesc = CreateTupleDescCopy(SPI_tuptable->tupdesc); - rc = routine->open_writer(path, tupdesc, &options, &writer); + rc = routine->open_writer((DatalakeFileSystem) open_fs, relative, + tupdesc, &options, &writer); if (rc != DL_OK) dl_error_report(ERROR, rc, "open_writer"); open_writer = writer; @@ -289,11 +460,13 @@ datalake_parquet_write(PG_FUNCTION_ARGS) { DlArrowBuilder builder = open_builder; FormatWriter *writer = open_writer; + DatalakeFileSystem fs = (DatalakeFileSystem) open_fs; if (builder != NULL) dl_arrow_builder_close(&builder); if (writer != NULL) writer->ops->abort(&writer); + datalake_fs_close(&fs); PG_RE_THROW(); } @@ -301,6 +474,12 @@ datalake_parquet_write(PG_FUNCTION_ARGS) SPI_finish(); + { + DatalakeFileSystem fs = (DatalakeFileSystem) open_fs; + + datalake_fs_close(&fs); + } + PG_RETURN_INT64(written); } @@ -344,15 +523,27 @@ datalake_parquet_read(PG_FUNCTION_ARGS) Datum *values; bool *nulls; FormatReader *reader = NULL; + DatalakeFileSystem volatile open_fs = NULL; + char *relative; MemoryContext row_context; DlErrCode rc; - check_nargs(fcinfo, 4); + check_nargs(fcinfo, 5); + /* Not STRICT, because volume is optional; the rest are not. */ + if (PG_ARGISNULL(0) || PG_ARGISNULL(1) || PG_ARGISNULL(2) || PG_ARGISNULL(3)) + ereport(ERROR, + (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED), + errmsg("path, first_row_group, n_row_groups and field_ids are required"))); path = text_to_cstring(PG_GETARG_TEXT_PP(0)); field_id_array = PG_GETARG_ARRAYTYPE_P(3); routine = parquet_routine(); + open_fs = storage_open_volume(path, + PG_ARGISNULL(4) ? NULL : + text_to_cstring(PG_GETARG_TEXT_PP(4)), + true, &relative); - fragment.path = path; + fragment.fs = (DatalakeFileSystem) open_fs; + fragment.path = relative; fragment.first_row_group = PG_GETARG_INT32(1); fragment.n_row_groups = PG_GETARG_INT32(2); @@ -474,6 +665,7 @@ datalake_parquet_read(PG_FUNCTION_ARGS) PG_CATCH(); { FormatReader *failed = open_reader; + DatalakeFileSystem fs = (DatalakeFileSystem) open_fs; if (batch->release != NULL) batch->release(batch); @@ -481,10 +673,273 @@ datalake_parquet_read(PG_FUNCTION_ARGS) schema->release(schema); if (failed != NULL) failed->ops->close(&failed); + datalake_fs_close(&fs); + + PG_RE_THROW(); + } + PG_END_TRY(); + + { + DatalakeFileSystem fs = (DatalakeFileSystem) open_fs; + + datalake_fs_close(&fs); + } + + return (Datum) 0; +} + +/* + * The handles an error has to release live in volatile locals, which is what + * lets PG_CATCH still read them after a longjmp; same shape as the Parquet + * writer above. Each is cleared as soon as something else owns it. + */ +Datum +datalake_storage_write_text(PG_FUNCTION_ARGS) +{ + text *content; + char *relative; + DatalakeFileSystem volatile open_fs = NULL; + DatalakeFile volatile open_file = NULL; + int64 length; + + check_nargs(fcinfo, 3); + if (PG_ARGISNULL(0) || PG_ARGISNULL(1)) + PG_RETURN_NULL(); + content = PG_GETARG_TEXT_PP(1); + length = VARSIZE_ANY_EXHDR(content); + open_fs = storage_open_uri(fcinfo, 0, 2, true, &relative); + + PG_TRY(); + { + DatalakeFileSystem fs = (DatalakeFileSystem) open_fs; + DatalakeFile file = NULL; + DlErrCode rc; + + rc = datalake_file_open(fs, relative, DATALAKE_FILE_WRITE, &file); + open_file = file; + if (rc == DL_OK) + rc = datalake_file_write(file, VARDATA_ANY(content), length); + if (rc == DL_OK) + { + rc = datalake_file_close(&file); + open_file = file; /* close consumes the handle */ + } + if (rc != DL_OK) + dl_error_report(ERROR, rc, "write storage file"); + } + PG_CATCH(); + { + DatalakeFile file = (DatalakeFile) open_file; + DatalakeFileSystem fs = (DatalakeFileSystem) open_fs; + + datalake_file_abort(&file); + datalake_fs_close(&fs); + PG_RE_THROW(); + } + PG_END_TRY(); + { + DatalakeFileSystem fs = (DatalakeFileSystem) open_fs; + + datalake_fs_close(&fs); + } + PG_RETURN_INT64(length); +} + +Datum +datalake_storage_read_text(PG_FUNCTION_ARGS) +{ + char *relative; + DatalakeFileSystem volatile open_fs = NULL; + DatalakeFile volatile open_file = NULL; + StringInfoData data; + + check_nargs(fcinfo, 2); + if (PG_ARGISNULL(0)) + PG_RETURN_NULL(); + open_fs = storage_open_uri(fcinfo, 0, 1, true, &relative); + initStringInfo(&data); + PG_TRY(); + { + DatalakeFileSystem fs = (DatalakeFileSystem) open_fs; + DatalakeFile file = NULL; + char buffer[8192]; + DlErrCode rc; + + rc = datalake_file_open(fs, relative, DATALAKE_FILE_READ, &file); + open_file = file; + while (rc == DL_OK) + { + int64 nread; + + rc = datalake_file_read(file, buffer, sizeof(buffer), &nread); + if (rc != DL_OK || nread == 0) + break; + appendBinaryStringInfo(&data, buffer, nread); + } + if (rc == DL_OK) + { + rc = datalake_file_close(&file); + open_file = file; /* close consumes the handle */ + } + if (rc != DL_OK) + dl_error_report(ERROR, rc, "read storage file"); + } + PG_CATCH(); + { + DatalakeFile file = (DatalakeFile) open_file; + DatalakeFileSystem fs = (DatalakeFileSystem) open_fs; + + datalake_file_abort(&file); + datalake_fs_close(&fs); PG_RE_THROW(); } PG_END_TRY(); + { + DatalakeFileSystem fs = (DatalakeFileSystem) open_fs; + + datalake_fs_close(&fs); + } + PG_RETURN_TEXT_P(cstring_to_text_with_len(data.data, data.len)); +} + +Datum +datalake_storage_list(PG_FUNCTION_ARGS) +{ + char *relative; + DatalakeFileSystem volatile open_fs = NULL; + char **volatile open_names = NULL; + int volatile open_nnames = 0; + ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo; + + check_nargs(fcinfo, 2); + if (PG_ARGISNULL(0)) + PG_RETURN_NULL(); + open_fs = storage_open_uri(fcinfo, 0, 1, false, &relative); + PG_TRY(); + { + DatalakeFileSystem fs = (DatalakeFileSystem) open_fs; + char **names = NULL; + int nnames = 0; + int i; + DlErrCode rc; + + rc = datalake_fs_list(fs, relative, &names, &nnames); + open_names = names; + open_nnames = nnames; + if (rc != DL_OK) + dl_error_report(ERROR, rc, "list storage"); + InitMaterializedSRF(fcinfo, MAT_SRF_USE_EXPECTED_DESC); + for (i = 0; i < nnames; i++) + { + Datum value = CStringGetTextDatum(names[i]); + bool isnull = false; + + tuplestore_putvalues(rsinfo->setResult, rsinfo->setDesc, + &value, &isnull); + free(names[i]); + names[i] = NULL; /* so the cleanup path cannot free it twice */ + } + free(names); + open_names = NULL; + open_nnames = 0; + } + PG_CATCH(); + { + char **names = (char **) open_names; + DatalakeFileSystem fs = (DatalakeFileSystem) open_fs; + int i; + + for (i = 0; names != NULL && i < open_nnames; i++) + free(names[i]); + free(names); + datalake_fs_close(&fs); + PG_RE_THROW(); + } + PG_END_TRY(); + + { + DatalakeFileSystem fs = (DatalakeFileSystem) open_fs; + + datalake_fs_close(&fs); + } return (Datum) 0; } + +Datum +datalake_storage_delete(PG_FUNCTION_ARGS) +{ + char *relative; + DatalakeFileSystem volatile open_fs = NULL; + + check_nargs(fcinfo, 2); + if (PG_ARGISNULL(0)) + PG_RETURN_NULL(); + open_fs = storage_open_uri(fcinfo, 0, 1, true, &relative); + + PG_TRY(); + { + DatalakeFileSystem fs = (DatalakeFileSystem) open_fs; + DlErrCode rc = datalake_file_delete(fs, relative); + + if (rc != DL_OK) + dl_error_report(ERROR, rc, "delete storage file"); + } + PG_CATCH(); + { + DatalakeFileSystem fs = (DatalakeFileSystem) open_fs; + + datalake_fs_close(&fs); + PG_RE_THROW(); + } + PG_END_TRY(); + + { + DatalakeFileSystem fs = (DatalakeFileSystem) open_fs; + + datalake_fs_close(&fs); + } + PG_RETURN_BOOL(true); +} + +Datum +datalake_storage_probe(PG_FUNCTION_ARGS) +{ + DatalakeLocation location = {0}; + DatalakeFileSystem fs = NULL; + char *scheme; + DlErrCode rc; + + check_nargs(fcinfo, 1); + if (PG_ARGISNULL(0)) + PG_RETURN_NULL(); + scheme = text_to_cstring(PG_GETARG_TEXT_PP(0)); + location.abi_version = DATALAKE_LOCATION_ABI_VERSION; + location.scheme = scheme; + location.authority = pstrdup(strcmp(scheme, "s3") == 0 ? "probe-bucket" : ""); + location.path_prefix = pstrdup("/tmp"); + rc = datalake_fs_open(&location, NULL, 0, &fs); + if (rc == DL_OK) + { + datalake_fs_close(&fs); + PG_RETURN_TEXT_P(cstring_to_text("supported")); + } + PG_RETURN_TEXT_P(cstring_to_text(dl_error_get()->message)); +} + +Datum +datalake_storage_register_bad(PG_FUNCTION_ARGS) +{ + char *kind; + DlErrCode rc; + + check_nargs(fcinfo, 1); + if (PG_ARGISNULL(0)) + PG_RETURN_NULL(); + kind = text_to_cstring(PG_GETARG_TEXT_PP(0)); + rc = datalake_test_register_bad_storage_backend(kind); + if (rc == DL_OK) + PG_RETURN_TEXT_P(cstring_to_text("accepted")); + PG_RETURN_TEXT_P(cstring_to_text(dl_error_get()->message)); +} diff --git a/contrib/datalake_fdw/src/test/storage_test_backend.cpp b/contrib/datalake_fdw/src/test/storage_test_backend.cpp new file mode 100644 index 00000000000..41e47275249 --- /dev/null +++ b/contrib/datalake_fdw/src/test/storage_test_backend.cpp @@ -0,0 +1,98 @@ +/*------------------------------------------------------------------------- + * + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * + * storage_test_backend.cpp + * A storage backend that exists so the registration contract can be + * tested from inside this module. + * + * IDENTIFICATION + * contrib/datalake_fdw/src/test/storage_test_backend.cpp + * + *------------------------------------------------------------------------- + */ + +#include + +#include +#include +#include + +#include "common/local_file_system.h" +#include "common/storage_backend.h" + +/* + * A second backend registered through the public contract, so the storage + * cases prove that a backend which is not the built-in one works the same + * way. It mounts the same create-only local file system under a subtree, + * which keeps one implementation of the write and abort semantics rather + * than a second one that could drift. + */ +static arrow::Result +mount_dltest(const DatalakeLocation *location, const DlKeyValue *, int, + const DatalakeStorageHost *host) +{ + if (location == NULL || location->path_prefix == NULL) + return arrow::Status::Invalid("dltest location has no path"); + + auto local = std::make_shared( + arrow::io::IOContext(dl_storage_host_pool(host))); + DatalakeMountedFs mounted; + + mounted.fs = std::make_shared( + location->path_prefix, local); + mounted.root = ""; + return mounted; +} + +static const DatalakeStorageBackend dltest_backend = { + DL_STORAGE_ABI_VERSION, sizeof(DatalakeStorageBackend), "dltest", + ARROW_VERSION_STRING, DL_STORAGE_ABI_FINGERPRINT, mount_dltest, NULL, NULL +}; + +extern "C" DlErrCode +datalake_register_test_storage_backend(void) +{ + return datalake_register_storage_backend(&dltest_backend); +} + +/* + * Each kind breaks exactly one of the registration checks, so a case can + * assert the message that check produces. The scheme stays "dltest", which + * is already registered: a check that stopped working would fall through to + * the duplicate-scheme rejection, and the cases tell those apart by naming + * the field and the value they expect to see reported. + */ +extern "C" DlErrCode +datalake_test_register_bad_storage_backend(const char *kind) +{ + DatalakeStorageBackend bad = dltest_backend; + + if (strcmp(kind, "abi_version") == 0) + bad.abi_version++; + else if (strcmp(kind, "struct_size") == 0) + bad.struct_size = 0; + else if (strcmp(kind, "arrow_version") == 0) + bad.arrow_version = "0.0.0-test"; + else if (strcmp(kind, "abi_fingerprint") == 0) + bad.abi_fingerprint = "gcc0;cxx11abi=9;arrow=0.0.0-test"; + else if (strcmp(kind, "duplicate") != 0) + return DL_ARG_ERROR("register bad storage backend"); + + return datalake_register_storage_backend(&bad); +} diff --git a/contrib/datalake_fdw/test/automation/sqlrepo/smoke/iceberg_am/expected/iceberg_am_reject.out b/contrib/datalake_fdw/test/automation/sqlrepo/smoke/iceberg_am/expected/iceberg_am_reject.out index d6974b509f7..ea09037e381 100644 --- a/contrib/datalake_fdw/test/automation/sqlrepo/smoke/iceberg_am/expected/iceberg_am_reject.out +++ b/contrib/datalake_fdw/test/automation/sqlrepo/smoke/iceberg_am/expected/iceberg_am_reject.out @@ -417,7 +417,7 @@ CREATE SERVER dlskel_bad_scheme FOREIGN DATA WRAPPER iceberg_volume_fdw OPTIONS (base_path 'ftp://x/y'); ERROR: invalid iceberg volume base_path "ftp://x/y" -DETAIL: location URI "ftp://x/y" has unsupported scheme; expected s3 or hdfs +DETAIL: location URI "ftp://x/y" names storage "ftp", which no backend is registered for CREATE SERVER dlskel_bad_authority FOREIGN DATA WRAPPER iceberg_volume_fdw OPTIONS (base_path 's3://'); @@ -426,13 +426,13 @@ DETAIL: location URI "s3://" has an empty authority CREATE SERVER dlskel_bad_query FOREIGN DATA WRAPPER iceberg_volume_fdw OPTIONS (base_path 's3://b/p?versionId=3'); -ERROR: invalid iceberg volume base_path "s3://b/p?versionId=3" -DETAIL: location URI "s3://b/p?versionId=3" must not contain a query +ERROR: invalid iceberg volume base_path "s3://b/p?***" +DETAIL: location URI must not contain a query CREATE SERVER dlskel_bad_userinfo FOREIGN DATA WRAPPER iceberg_volume_fdw OPTIONS (base_path 's3://user@b/p'); -ERROR: invalid iceberg volume base_path "s3://user@b/p" -DETAIL: location URI "s3://user@b/p" must not contain userinfo +ERROR: invalid iceberg volume base_path "s3://***@b/p" +DETAIL: location URI must not contain userinfo CREATE SERVER dlskel_bad_bucket FOREIGN DATA WRAPPER iceberg_volume_fdw OPTIONS (base_path 's3://UPPER_case/p'); diff --git a/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_conformance/expected/conformance_dltest.out b/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_conformance/expected/conformance_dltest.out new file mode 100644 index 00000000000..6de85c2788a --- /dev/null +++ b/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_conformance/expected/conformance_dltest.out @@ -0,0 +1,159 @@ +-- A backend registered from outside the module through the public contract, +-- mounted under a subtree. Its paths are relative rather than absolute, which +-- is exactly the difference a backend is allowed to have. +SET client_min_messages = warning; +CREATE EXTENSION IF NOT EXISTS datalake_fdw; +CREATE EXTENSION IF NOT EXISTS datalake_fdw_test; +RESET client_min_messages; +COPY (SELECT 1) TO PROGRAM + 'rm -rf /tmp/datalake_fdw_conformance_dltest && mkdir -p /tmp/datalake_fdw_conformance_dltest'; +\set prefix 'dltest:///tmp/datalake_fdw_conformance_dltest' +\set root '' +\set volume NULL +\set kv NULL +-- pg_regress feeds the script to psql on standard input, so there is no +-- script directory for \ir to resolve against; the path is relative to where +-- make runs, which is the module's own directory. +\i test/automation/sqlrepo/smoke/storage_conformance/sql/conformance_body.sql +-- The storage behaviour every backend owes the format layer, written once and +-- run against each of them. Included by conformance_file, conformance_dltest +-- and conformance_s3, which differ only in where :prefix points, whether a +-- :volume supplies credentials, and the :kv the text helpers need. +-- +-- What is asserted here is what changes with the storage underneath: that a +-- Parquet file written through a volume reads back, that a name in use is +-- never overwritten, and that a write which fails partway leaves nothing. +-- What a Parquet file holds is the business of the format_parquet cases. +SELECT datalake_parquet_write(:'prefix' || '/roundtrip.parquet', + $q$SELECT i AS id, (i * 1.5)::float8 AS amount, 'row ' || i AS label, + (i % 2 = 0) AS flag, ('2024-01-01'::date + i) AS day, + ('2024-01-01 00:00:00+00'::timestamptz + i * interval '1 second') AS at, + decode(lpad(to_hex(i), 8, '0'), 'hex') AS raw + FROM generate_series(1, 2000) i$q$, + 500, 'snappy', :volume) AS rows_written; + rows_written +-------------- + 2000 +(1 row) + +SELECT count(*) AS rows_read, + sum(id) AS id_sum, + min(label) AS first_label, + count(*) FILTER (WHERE flag) AS flagged, + max(day) AS last_day, + max(at) AS last_at, + max(encode(raw, 'hex')) AS last_raw +FROM datalake_parquet_read(:'prefix' || '/roundtrip.parquet', 0, 0, '{}', :volume) + AS t(id int, amount float8, label text, flag boolean, day date, + at timestamptz, raw bytea); + rows_read | id_sum | first_label | flagged | last_day | last_at | last_raw +-----------+---------+-------------+---------+------------+------------------------------+---------- + 2000 | 2001000 | row 1 | 1000 | 06-23-2029 | Sun Dec 31 16:33:20 2023 PST | 000007d0 +(1 row) + +-- Columns are matched by the field id each one carries, not by where it sits +-- in the file, so a projection can name them in any order and leave some out. +SELECT count(*) AS projected_rows, min(label) AS first_label, sum(id) AS id_sum +FROM datalake_parquet_read(:'prefix' || '/roundtrip.parquet', 0, 0, + '{3,1}', :volume) AS t(label text, id int); + projected_rows | first_label | id_sum +----------------+-------------+--------- + 2000 | row 1 | 2001000 +(1 row) + +-- Written with 500-row groups, so a range of them is a range of the file. +SELECT count(*) AS rows_in_two_groups, min(id) AS first_id, max(id) AS last_id +FROM datalake_parquet_read(:'prefix' || '/roundtrip.parquet', 1, 2, '{1}', :volume) + AS t(id int); + rows_in_two_groups | first_id | last_id +--------------------+----------+--------- + 1000 | 501 | 1500 +(1 row) + +-- The file is there, and it is the only thing there. +SELECT replace(path, :'root', '') AS object +FROM datalake_storage_list(:'prefix', :kv) AS path +ORDER BY 1; + object +------------------- + roundtrip.parquet +(1 row) + +-- A name in use is refused, and what was there is untouched. +\set VERBOSITY sqlstate +SELECT datalake_parquet_write(:'prefix' || '/roundtrip.parquet', + 'SELECT 1 AS id', 0, '', :volume); +ERROR: 42P07 +\set VERBOSITY default +SELECT count(*) AS rows_still_there +FROM datalake_parquet_read(:'prefix' || '/roundtrip.parquet', 0, 0, '{1}', :volume) + AS t(id int); + rows_still_there +------------------ + 2000 +(1 row) + +-- A write that fails partway leaves nothing behind: not a truncated file, not +-- an empty one, and nothing for the next attempt at that name to trip over. +\set VERBOSITY sqlstate +-- Each row carries a kilobyte, and the failure comes after twelve thousand of +-- them: past the point where object storage has begun a multipart upload, so +-- what this proves is that the upload is abandoned and not merely that no +-- object appears. +SELECT datalake_parquet_write(:'prefix' || '/aborted.parquet', + $q$SELECT i AS id, repeat('x', 1024) AS padding, + 1 / (i - 12000) AS boom + FROM generate_series(1, 20000) i$q$, + 2000, '', :volume); +ERROR: 22012 +\set VERBOSITY default +SELECT replace(path, :'root', '') AS object +FROM datalake_storage_list(:'prefix', :kv) AS path +ORDER BY 1; + object +------------------- + roundtrip.parquet +(1 row) + +-- And the name is free, so the next writer gets it. +SELECT datalake_parquet_write(:'prefix' || '/aborted.parquet', + 'SELECT 7 AS id', 0, '', :volume) AS rows_written; + rows_written +-------------- + 1 +(1 row) + +SELECT id FROM datalake_parquet_read(:'prefix' || '/aborted.parquet', 0, 0, '{}', + :volume) AS t(id int); + id +---- + 7 +(1 row) + +-- Reading something that is not there says so, whatever the storage is. +\set VERBOSITY sqlstate +SELECT count(*) FROM datalake_parquet_read(:'prefix' || '/missing.parquet', 0, 0, + '{}', :volume) AS t(id int); +ERROR: 42704 +-- So does listing a prefix nothing was ever written under. Object storage has +-- no such thing as a directory and would answer an empty list; a filesystem +-- would answer that there is no such directory. The rule is the facade's, so +-- both say the same thing here. +SELECT count(*) FROM datalake_storage_list(:'prefix' || '/never-written', :kv); +ERROR: 42704 +\set VERBOSITY default +SELECT datalake_storage_delete(:'prefix' || '/roundtrip.parquet', :kv) AS cleaned_roundtrip, + datalake_storage_delete(:'prefix' || '/aborted.parquet', :kv) AS cleaned_aborted; + cleaned_roundtrip | cleaned_aborted +-------------------+----------------- + t | t +(1 row) + +-- And a prefix that held objects until a moment ago is no different from one +-- that never did: on a filesystem the directory is still there and empty, and +-- that has to read the same way. +\set VERBOSITY sqlstate +SELECT count(*) FROM datalake_storage_list(:'prefix', :kv); +ERROR: 42704 +\set VERBOSITY default +COPY (SELECT 1) TO PROGRAM 'rm -rf /tmp/datalake_fdw_conformance_dltest'; diff --git a/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_conformance/expected/conformance_file.out b/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_conformance/expected/conformance_file.out new file mode 100644 index 00000000000..de4c2e45dbf --- /dev/null +++ b/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_conformance/expected/conformance_file.out @@ -0,0 +1,158 @@ +-- The built-in file backend: a shared mount, which is what a volume on a +-- cluster filesystem looks like to this layer. +SET client_min_messages = warning; +CREATE EXTENSION IF NOT EXISTS datalake_fdw; +CREATE EXTENSION IF NOT EXISTS datalake_fdw_test; +RESET client_min_messages; +COPY (SELECT 1) TO PROGRAM + 'rm -rf /tmp/datalake_fdw_conformance_file && mkdir -p /tmp/datalake_fdw_conformance_file'; +\set prefix 'file:///tmp/datalake_fdw_conformance_file' +\set root '/tmp/datalake_fdw_conformance_file/' +\set volume NULL +\set kv NULL +-- pg_regress feeds the script to psql on standard input, so there is no +-- script directory for \ir to resolve against; the path is relative to where +-- make runs, which is the module's own directory. +\i test/automation/sqlrepo/smoke/storage_conformance/sql/conformance_body.sql +-- The storage behaviour every backend owes the format layer, written once and +-- run against each of them. Included by conformance_file, conformance_dltest +-- and conformance_s3, which differ only in where :prefix points, whether a +-- :volume supplies credentials, and the :kv the text helpers need. +-- +-- What is asserted here is what changes with the storage underneath: that a +-- Parquet file written through a volume reads back, that a name in use is +-- never overwritten, and that a write which fails partway leaves nothing. +-- What a Parquet file holds is the business of the format_parquet cases. +SELECT datalake_parquet_write(:'prefix' || '/roundtrip.parquet', + $q$SELECT i AS id, (i * 1.5)::float8 AS amount, 'row ' || i AS label, + (i % 2 = 0) AS flag, ('2024-01-01'::date + i) AS day, + ('2024-01-01 00:00:00+00'::timestamptz + i * interval '1 second') AS at, + decode(lpad(to_hex(i), 8, '0'), 'hex') AS raw + FROM generate_series(1, 2000) i$q$, + 500, 'snappy', :volume) AS rows_written; + rows_written +-------------- + 2000 +(1 row) + +SELECT count(*) AS rows_read, + sum(id) AS id_sum, + min(label) AS first_label, + count(*) FILTER (WHERE flag) AS flagged, + max(day) AS last_day, + max(at) AS last_at, + max(encode(raw, 'hex')) AS last_raw +FROM datalake_parquet_read(:'prefix' || '/roundtrip.parquet', 0, 0, '{}', :volume) + AS t(id int, amount float8, label text, flag boolean, day date, + at timestamptz, raw bytea); + rows_read | id_sum | first_label | flagged | last_day | last_at | last_raw +-----------+---------+-------------+---------+------------+------------------------------+---------- + 2000 | 2001000 | row 1 | 1000 | 06-23-2029 | Sun Dec 31 16:33:20 2023 PST | 000007d0 +(1 row) + +-- Columns are matched by the field id each one carries, not by where it sits +-- in the file, so a projection can name them in any order and leave some out. +SELECT count(*) AS projected_rows, min(label) AS first_label, sum(id) AS id_sum +FROM datalake_parquet_read(:'prefix' || '/roundtrip.parquet', 0, 0, + '{3,1}', :volume) AS t(label text, id int); + projected_rows | first_label | id_sum +----------------+-------------+--------- + 2000 | row 1 | 2001000 +(1 row) + +-- Written with 500-row groups, so a range of them is a range of the file. +SELECT count(*) AS rows_in_two_groups, min(id) AS first_id, max(id) AS last_id +FROM datalake_parquet_read(:'prefix' || '/roundtrip.parquet', 1, 2, '{1}', :volume) + AS t(id int); + rows_in_two_groups | first_id | last_id +--------------------+----------+--------- + 1000 | 501 | 1500 +(1 row) + +-- The file is there, and it is the only thing there. +SELECT replace(path, :'root', '') AS object +FROM datalake_storage_list(:'prefix', :kv) AS path +ORDER BY 1; + object +------------------- + roundtrip.parquet +(1 row) + +-- A name in use is refused, and what was there is untouched. +\set VERBOSITY sqlstate +SELECT datalake_parquet_write(:'prefix' || '/roundtrip.parquet', + 'SELECT 1 AS id', 0, '', :volume); +ERROR: 42P07 +\set VERBOSITY default +SELECT count(*) AS rows_still_there +FROM datalake_parquet_read(:'prefix' || '/roundtrip.parquet', 0, 0, '{1}', :volume) + AS t(id int); + rows_still_there +------------------ + 2000 +(1 row) + +-- A write that fails partway leaves nothing behind: not a truncated file, not +-- an empty one, and nothing for the next attempt at that name to trip over. +\set VERBOSITY sqlstate +-- Each row carries a kilobyte, and the failure comes after twelve thousand of +-- them: past the point where object storage has begun a multipart upload, so +-- what this proves is that the upload is abandoned and not merely that no +-- object appears. +SELECT datalake_parquet_write(:'prefix' || '/aborted.parquet', + $q$SELECT i AS id, repeat('x', 1024) AS padding, + 1 / (i - 12000) AS boom + FROM generate_series(1, 20000) i$q$, + 2000, '', :volume); +ERROR: 22012 +\set VERBOSITY default +SELECT replace(path, :'root', '') AS object +FROM datalake_storage_list(:'prefix', :kv) AS path +ORDER BY 1; + object +------------------- + roundtrip.parquet +(1 row) + +-- And the name is free, so the next writer gets it. +SELECT datalake_parquet_write(:'prefix' || '/aborted.parquet', + 'SELECT 7 AS id', 0, '', :volume) AS rows_written; + rows_written +-------------- + 1 +(1 row) + +SELECT id FROM datalake_parquet_read(:'prefix' || '/aborted.parquet', 0, 0, '{}', + :volume) AS t(id int); + id +---- + 7 +(1 row) + +-- Reading something that is not there says so, whatever the storage is. +\set VERBOSITY sqlstate +SELECT count(*) FROM datalake_parquet_read(:'prefix' || '/missing.parquet', 0, 0, + '{}', :volume) AS t(id int); +ERROR: 42704 +-- So does listing a prefix nothing was ever written under. Object storage has +-- no such thing as a directory and would answer an empty list; a filesystem +-- would answer that there is no such directory. The rule is the facade's, so +-- both say the same thing here. +SELECT count(*) FROM datalake_storage_list(:'prefix' || '/never-written', :kv); +ERROR: 42704 +\set VERBOSITY default +SELECT datalake_storage_delete(:'prefix' || '/roundtrip.parquet', :kv) AS cleaned_roundtrip, + datalake_storage_delete(:'prefix' || '/aborted.parquet', :kv) AS cleaned_aborted; + cleaned_roundtrip | cleaned_aborted +-------------------+----------------- + t | t +(1 row) + +-- And a prefix that held objects until a moment ago is no different from one +-- that never did: on a filesystem the directory is still there and empty, and +-- that has to read the same way. +\set VERBOSITY sqlstate +SELECT count(*) FROM datalake_storage_list(:'prefix', :kv); +ERROR: 42704 +\set VERBOSITY default +COPY (SELECT 1) TO PROGRAM 'rm -rf /tmp/datalake_fdw_conformance_file'; diff --git a/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_conformance/expected/conformance_s3.out b/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_conformance/expected/conformance_s3.out new file mode 100644 index 00000000000..e3c9def9a75 --- /dev/null +++ b/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_conformance/expected/conformance_s3.out @@ -0,0 +1,169 @@ +-- The s3 backend, reached the way a user reaches it: through a volume server +-- and a user mapping, rather than by handing credentials to a function. +SET client_min_messages = warning; +CREATE EXTENSION IF NOT EXISTS datalake_fdw; +CREATE EXTENSION IF NOT EXISTS datalake_fdw_test; +RESET client_min_messages; +SET log_statement = 'none'; +SET log_min_duration_statement = -1; +SET log_min_error_statement = 'panic'; +-- Endpoint, bucket and credentials differ from one machine to the next, so +-- they are read with the echo off: what this file asserts must not depend on +-- where it ran. +\set ECHO none +NOTICE: server "dlconf_volume" does not exist, skipping +-- The volume exists and names this run's prefix. +SELECT count(*) = 1 AS volume_created +FROM pg_foreign_server WHERE srvname = 'dlconf_volume'; + volume_created +---------------- + t +(1 row) + +-- pg_regress feeds the script to psql on standard input, so there is no +-- script directory for \ir to resolve against; the path is relative to where +-- make runs, which is the module's own directory. +\i test/automation/sqlrepo/smoke/storage_conformance/sql/conformance_body.sql +-- The storage behaviour every backend owes the format layer, written once and +-- run against each of them. Included by conformance_file, conformance_dltest +-- and conformance_s3, which differ only in where :prefix points, whether a +-- :volume supplies credentials, and the :kv the text helpers need. +-- +-- What is asserted here is what changes with the storage underneath: that a +-- Parquet file written through a volume reads back, that a name in use is +-- never overwritten, and that a write which fails partway leaves nothing. +-- What a Parquet file holds is the business of the format_parquet cases. +SELECT datalake_parquet_write(:'prefix' || '/roundtrip.parquet', + $q$SELECT i AS id, (i * 1.5)::float8 AS amount, 'row ' || i AS label, + (i % 2 = 0) AS flag, ('2024-01-01'::date + i) AS day, + ('2024-01-01 00:00:00+00'::timestamptz + i * interval '1 second') AS at, + decode(lpad(to_hex(i), 8, '0'), 'hex') AS raw + FROM generate_series(1, 2000) i$q$, + 500, 'snappy', :volume) AS rows_written; + rows_written +-------------- + 2000 +(1 row) + +SELECT count(*) AS rows_read, + sum(id) AS id_sum, + min(label) AS first_label, + count(*) FILTER (WHERE flag) AS flagged, + max(day) AS last_day, + max(at) AS last_at, + max(encode(raw, 'hex')) AS last_raw +FROM datalake_parquet_read(:'prefix' || '/roundtrip.parquet', 0, 0, '{}', :volume) + AS t(id int, amount float8, label text, flag boolean, day date, + at timestamptz, raw bytea); + rows_read | id_sum | first_label | flagged | last_day | last_at | last_raw +-----------+---------+-------------+---------+------------+------------------------------+---------- + 2000 | 2001000 | row 1 | 1000 | 06-23-2029 | Sun Dec 31 16:33:20 2023 PST | 000007d0 +(1 row) + +-- Columns are matched by the field id each one carries, not by where it sits +-- in the file, so a projection can name them in any order and leave some out. +SELECT count(*) AS projected_rows, min(label) AS first_label, sum(id) AS id_sum +FROM datalake_parquet_read(:'prefix' || '/roundtrip.parquet', 0, 0, + '{3,1}', :volume) AS t(label text, id int); + projected_rows | first_label | id_sum +----------------+-------------+--------- + 2000 | row 1 | 2001000 +(1 row) + +-- Written with 500-row groups, so a range of them is a range of the file. +SELECT count(*) AS rows_in_two_groups, min(id) AS first_id, max(id) AS last_id +FROM datalake_parquet_read(:'prefix' || '/roundtrip.parquet', 1, 2, '{1}', :volume) + AS t(id int); + rows_in_two_groups | first_id | last_id +--------------------+----------+--------- + 1000 | 501 | 1500 +(1 row) + +-- The file is there, and it is the only thing there. +SELECT replace(path, :'root', '') AS object +FROM datalake_storage_list(:'prefix', :kv) AS path +ORDER BY 1; + object +------------------- + roundtrip.parquet +(1 row) + +-- A name in use is refused, and what was there is untouched. +\set VERBOSITY sqlstate +SELECT datalake_parquet_write(:'prefix' || '/roundtrip.parquet', + 'SELECT 1 AS id', 0, '', :volume); +ERROR: 42P07 +\set VERBOSITY default +SELECT count(*) AS rows_still_there +FROM datalake_parquet_read(:'prefix' || '/roundtrip.parquet', 0, 0, '{1}', :volume) + AS t(id int); + rows_still_there +------------------ + 2000 +(1 row) + +-- A write that fails partway leaves nothing behind: not a truncated file, not +-- an empty one, and nothing for the next attempt at that name to trip over. +\set VERBOSITY sqlstate +-- Each row carries a kilobyte, and the failure comes after twelve thousand of +-- them: past the point where object storage has begun a multipart upload, so +-- what this proves is that the upload is abandoned and not merely that no +-- object appears. +SELECT datalake_parquet_write(:'prefix' || '/aborted.parquet', + $q$SELECT i AS id, repeat('x', 1024) AS padding, + 1 / (i - 12000) AS boom + FROM generate_series(1, 20000) i$q$, + 2000, '', :volume); +ERROR: 22012 +\set VERBOSITY default +SELECT replace(path, :'root', '') AS object +FROM datalake_storage_list(:'prefix', :kv) AS path +ORDER BY 1; + object +------------------- + roundtrip.parquet +(1 row) + +-- And the name is free, so the next writer gets it. +SELECT datalake_parquet_write(:'prefix' || '/aborted.parquet', + 'SELECT 7 AS id', 0, '', :volume) AS rows_written; + rows_written +-------------- + 1 +(1 row) + +SELECT id FROM datalake_parquet_read(:'prefix' || '/aborted.parquet', 0, 0, '{}', + :volume) AS t(id int); + id +---- + 7 +(1 row) + +-- Reading something that is not there says so, whatever the storage is. +\set VERBOSITY sqlstate +SELECT count(*) FROM datalake_parquet_read(:'prefix' || '/missing.parquet', 0, 0, + '{}', :volume) AS t(id int); +ERROR: 42704 +-- So does listing a prefix nothing was ever written under. Object storage has +-- no such thing as a directory and would answer an empty list; a filesystem +-- would answer that there is no such directory. The rule is the facade's, so +-- both say the same thing here. +SELECT count(*) FROM datalake_storage_list(:'prefix' || '/never-written', :kv); +ERROR: 42704 +\set VERBOSITY default +SELECT datalake_storage_delete(:'prefix' || '/roundtrip.parquet', :kv) AS cleaned_roundtrip, + datalake_storage_delete(:'prefix' || '/aborted.parquet', :kv) AS cleaned_aborted; + cleaned_roundtrip | cleaned_aborted +-------------------+----------------- + t | t +(1 row) + +-- And a prefix that held objects until a moment ago is no different from one +-- that never did: on a filesystem the directory is still there and empty, and +-- that has to read the same way. +\set VERBOSITY sqlstate +SELECT count(*) FROM datalake_storage_list(:'prefix', :kv); +ERROR: 42704 +\set VERBOSITY default +\set ECHO none +NOTICE: drop cascades to user mapping for gpadmin on server dlconf_volume diff --git a/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_conformance/expected/volume_resolve.out b/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_conformance/expected/volume_resolve.out new file mode 100644 index 00000000000..95c82fc51f3 --- /dev/null +++ b/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_conformance/expected/volume_resolve.out @@ -0,0 +1,118 @@ +-- Resolving a volume: who may use it, whose credentials are used, and what +-- happens when a path does not belong to it. A file volume needs no service +-- to talk to, so this runs everywhere. +SET client_min_messages = warning; +CREATE EXTENSION IF NOT EXISTS datalake_fdw; +CREATE EXTENSION IF NOT EXISTS datalake_fdw_test; +RESET client_min_messages; +COPY (SELECT 1) TO PROGRAM + 'rm -rf /tmp/datalake_fdw_volume && mkdir -p /tmp/datalake_fdw_volume/inside'; +DROP SERVER IF EXISTS dlvol CASCADE; +NOTICE: server "dlvol" does not exist, skipping +DROP SERVER IF EXISTS dlvol_other CASCADE; +NOTICE: server "dlvol_other" does not exist, skipping +DROP ROLE IF EXISTS dlvol_user; +CREATE ROLE dlvol_user LOGIN; +NOTICE: resource queue required -- using default resource queue "pg_default" +CREATE SERVER dlvol FOREIGN DATA WRAPPER iceberg_volume_fdw + OPTIONS (base_path 'file:///tmp/datalake_fdw_volume/inside'); +CREATE SERVER dlvol_other FOREIGN DATA WRAPPER iceberg_volume_fdw + OPTIONS (base_path 'file:///tmp/datalake_fdw_volume/elsewhere'); +-- Through the volume, as its owner. +SELECT datalake_parquet_write('file:///tmp/datalake_fdw_volume/inside/a.parquet', + 'SELECT 42 AS id', 0, '', 'dlvol') AS rows_written; + rows_written +-------------- + 1 +(1 row) + +SELECT id FROM datalake_parquet_read( + 'file:///tmp/datalake_fdw_volume/inside/a.parquet', 0, 0, '{}', 'dlvol') + AS t(id int); + id +---- + 42 +(1 row) + +-- A path outside the volume is refused, even though the volume would have +-- been happy to lend its settings to it. +SELECT datalake_parquet_write('file:///tmp/datalake_fdw_volume/outside.parquet', + 'SELECT 1 AS id', 0, '', 'dlvol'); +ERROR: "file:///tmp/datalake_fdw_volume/outside.parquet" is not inside volume "dlvol" +-- And so is one under a different volume. +SELECT datalake_parquet_write('file:///tmp/datalake_fdw_volume/inside/b.parquet', + 'SELECT 1 AS id', 0, '', 'dlvol_other'); +ERROR: "file:///tmp/datalake_fdw_volume/inside/b.parquet" is not inside volume "dlvol_other" +-- A volume that does not exist. +SELECT id FROM datalake_parquet_read( + 'file:///tmp/datalake_fdw_volume/inside/a.parquet', 0, 0, '{}', + 'no_such_volume') AS t(id int); +ERROR: server "no_such_volume" does not exist +-- Using a volume takes USAGE on it. +GRANT EXECUTE ON FUNCTION datalake_parquet_read(text, int, int, int[], text) + TO dlvol_user; +SET ROLE dlvol_user; +SELECT id FROM datalake_parquet_read( + 'file:///tmp/datalake_fdw_volume/inside/a.parquet', 0, 0, '{}', 'dlvol') + AS t(id int); +ERROR: permission denied for foreign server dlvol +RESET ROLE; +GRANT USAGE ON FOREIGN SERVER dlvol TO dlvol_user; +SET ROLE dlvol_user; +SELECT id FROM datalake_parquet_read( + 'file:///tmp/datalake_fdw_volume/inside/a.parquet', 0, 0, '{}', 'dlvol') + AS t(id int); + id +---- + 42 +(1 row) + +RESET ROLE; +-- A PUBLIC mapping is what a user without one of their own gets. The file +-- backend ignores credentials, so what is asserted is that resolution finds +-- the mapping and still reaches the file, not that the values did anything. +CREATE USER MAPPING FOR PUBLIC SERVER dlvol + OPTIONS (access_key_id 'public-key', secret_access_key 'public-secret-value'); +SET ROLE dlvol_user; +SELECT id FROM datalake_parquet_read( + 'file:///tmp/datalake_fdw_volume/inside/a.parquet', 0, 0, '{}', 'dlvol') + AS t(id int); + id +---- + 42 +(1 row) + +RESET ROLE; +CREATE USER MAPPING FOR dlvol_user SERVER dlvol + OPTIONS (access_key_id 'user-key', secret_access_key 'user-secret-value'); +SET ROLE dlvol_user; +SELECT id FROM datalake_parquet_read( + 'file:///tmp/datalake_fdw_volume/inside/a.parquet', 0, 0, '{}', 'dlvol') + AS t(id int); + id +---- + 42 +(1 row) + +RESET ROLE; +-- A base_path is quoted back when it is rejected, and a URI can carry a +-- password in its userinfo. Neither the message nor the detail may reproduce +-- one: if this case ever prints "hunter2", the rejection leaked a credential +-- into the server log. +CREATE SERVER dlvol_secret FOREIGN DATA WRAPPER iceberg_volume_fdw + OPTIONS (base_path 's3://reader:hunter2@bucket/prefix'); +ERROR: invalid iceberg volume base_path "s3://***@bucket/prefix" +DETAIL: location URI must not contain userinfo +CREATE SERVER dlvol_signed FOREIGN DATA WRAPPER iceberg_volume_fdw + OPTIONS (base_path 's3://bucket/prefix?X-Amz-Signature=deadbeef'); +ERROR: invalid iceberg volume base_path "s3://bucket/prefix?***" +DETAIL: location URI must not contain a query +DROP SERVER dlvol CASCADE; +NOTICE: drop cascades to 2 other objects +DETAIL: drop cascades to user mapping for public on server dlvol +drop cascades to user mapping for dlvol_user on server dlvol +DROP SERVER dlvol_other CASCADE; +DROP ROLE dlvol_user; +ERROR: role "dlvol_user" cannot be dropped because some objects depend on it +DETAIL: privileges for function datalake_parquet_read(text,integer,integer,integer[],text) +COPY (SELECT 1) TO PROGRAM 'rm -rf /tmp/datalake_fdw_volume'; diff --git a/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_conformance/sql/conformance_body.sql b/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_conformance/sql/conformance_body.sql new file mode 100644 index 00000000000..f5241f4a99a --- /dev/null +++ b/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_conformance/sql/conformance_body.sql @@ -0,0 +1,99 @@ +-- The storage behaviour every backend owes the format layer, written once and +-- run against each of them. Included by conformance_file, conformance_dltest +-- and conformance_s3, which differ only in where :prefix points, whether a +-- :volume supplies credentials, and the :kv the text helpers need. +-- +-- What is asserted here is what changes with the storage underneath: that a +-- Parquet file written through a volume reads back, that a name in use is +-- never overwritten, and that a write which fails partway leaves nothing. +-- What a Parquet file holds is the business of the format_parquet cases. + +SELECT datalake_parquet_write(:'prefix' || '/roundtrip.parquet', + $q$SELECT i AS id, (i * 1.5)::float8 AS amount, 'row ' || i AS label, + (i % 2 = 0) AS flag, ('2024-01-01'::date + i) AS day, + ('2024-01-01 00:00:00+00'::timestamptz + i * interval '1 second') AS at, + decode(lpad(to_hex(i), 8, '0'), 'hex') AS raw + FROM generate_series(1, 2000) i$q$, + 500, 'snappy', :volume) AS rows_written; + +SELECT count(*) AS rows_read, + sum(id) AS id_sum, + min(label) AS first_label, + count(*) FILTER (WHERE flag) AS flagged, + max(day) AS last_day, + max(at) AS last_at, + max(encode(raw, 'hex')) AS last_raw +FROM datalake_parquet_read(:'prefix' || '/roundtrip.parquet', 0, 0, '{}', :volume) + AS t(id int, amount float8, label text, flag boolean, day date, + at timestamptz, raw bytea); + +-- Columns are matched by the field id each one carries, not by where it sits +-- in the file, so a projection can name them in any order and leave some out. +SELECT count(*) AS projected_rows, min(label) AS first_label, sum(id) AS id_sum +FROM datalake_parquet_read(:'prefix' || '/roundtrip.parquet', 0, 0, + '{3,1}', :volume) AS t(label text, id int); + +-- Written with 500-row groups, so a range of them is a range of the file. +SELECT count(*) AS rows_in_two_groups, min(id) AS first_id, max(id) AS last_id +FROM datalake_parquet_read(:'prefix' || '/roundtrip.parquet', 1, 2, '{1}', :volume) + AS t(id int); + +-- The file is there, and it is the only thing there. +SELECT replace(path, :'root', '') AS object +FROM datalake_storage_list(:'prefix', :kv) AS path +ORDER BY 1; + +-- A name in use is refused, and what was there is untouched. +\set VERBOSITY sqlstate +SELECT datalake_parquet_write(:'prefix' || '/roundtrip.parquet', + 'SELECT 1 AS id', 0, '', :volume); +\set VERBOSITY default +SELECT count(*) AS rows_still_there +FROM datalake_parquet_read(:'prefix' || '/roundtrip.parquet', 0, 0, '{1}', :volume) + AS t(id int); + +-- A write that fails partway leaves nothing behind: not a truncated file, not +-- an empty one, and nothing for the next attempt at that name to trip over. +\set VERBOSITY sqlstate +-- Each row carries a kilobyte, and the failure comes after twelve thousand of +-- them: past the point where object storage has begun a multipart upload, so +-- what this proves is that the upload is abandoned and not merely that no +-- object appears. +SELECT datalake_parquet_write(:'prefix' || '/aborted.parquet', + $q$SELECT i AS id, repeat('x', 1024) AS padding, + 1 / (i - 12000) AS boom + FROM generate_series(1, 20000) i$q$, + 2000, '', :volume); +\set VERBOSITY default + +SELECT replace(path, :'root', '') AS object +FROM datalake_storage_list(:'prefix', :kv) AS path +ORDER BY 1; + +-- And the name is free, so the next writer gets it. +SELECT datalake_parquet_write(:'prefix' || '/aborted.parquet', + 'SELECT 7 AS id', 0, '', :volume) AS rows_written; +SELECT id FROM datalake_parquet_read(:'prefix' || '/aborted.parquet', 0, 0, '{}', + :volume) AS t(id int); + +-- Reading something that is not there says so, whatever the storage is. +\set VERBOSITY sqlstate +SELECT count(*) FROM datalake_parquet_read(:'prefix' || '/missing.parquet', 0, 0, + '{}', :volume) AS t(id int); + +-- So does listing a prefix nothing was ever written under. Object storage has +-- no such thing as a directory and would answer an empty list; a filesystem +-- would answer that there is no such directory. The rule is the facade's, so +-- both say the same thing here. +SELECT count(*) FROM datalake_storage_list(:'prefix' || '/never-written', :kv); +\set VERBOSITY default + +SELECT datalake_storage_delete(:'prefix' || '/roundtrip.parquet', :kv) AS cleaned_roundtrip, + datalake_storage_delete(:'prefix' || '/aborted.parquet', :kv) AS cleaned_aborted; + +-- And a prefix that held objects until a moment ago is no different from one +-- that never did: on a filesystem the directory is still there and empty, and +-- that has to read the same way. +\set VERBOSITY sqlstate +SELECT count(*) FROM datalake_storage_list(:'prefix', :kv); +\set VERBOSITY default diff --git a/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_conformance/sql/conformance_dltest.sql b/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_conformance/sql/conformance_dltest.sql new file mode 100644 index 00000000000..eca482cc747 --- /dev/null +++ b/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_conformance/sql/conformance_dltest.sql @@ -0,0 +1,22 @@ +-- A backend registered from outside the module through the public contract, +-- mounted under a subtree. Its paths are relative rather than absolute, which +-- is exactly the difference a backend is allowed to have. +SET client_min_messages = warning; +CREATE EXTENSION IF NOT EXISTS datalake_fdw; +CREATE EXTENSION IF NOT EXISTS datalake_fdw_test; +RESET client_min_messages; + +COPY (SELECT 1) TO PROGRAM + 'rm -rf /tmp/datalake_fdw_conformance_dltest && mkdir -p /tmp/datalake_fdw_conformance_dltest'; + +\set prefix 'dltest:///tmp/datalake_fdw_conformance_dltest' +\set root '' +\set volume NULL +\set kv NULL + +-- pg_regress feeds the script to psql on standard input, so there is no +-- script directory for \ir to resolve against; the path is relative to where +-- make runs, which is the module's own directory. +\i test/automation/sqlrepo/smoke/storage_conformance/sql/conformance_body.sql + +COPY (SELECT 1) TO PROGRAM 'rm -rf /tmp/datalake_fdw_conformance_dltest'; diff --git a/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_conformance/sql/conformance_file.sql b/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_conformance/sql/conformance_file.sql new file mode 100644 index 00000000000..71996426e29 --- /dev/null +++ b/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_conformance/sql/conformance_file.sql @@ -0,0 +1,21 @@ +-- The built-in file backend: a shared mount, which is what a volume on a +-- cluster filesystem looks like to this layer. +SET client_min_messages = warning; +CREATE EXTENSION IF NOT EXISTS datalake_fdw; +CREATE EXTENSION IF NOT EXISTS datalake_fdw_test; +RESET client_min_messages; + +COPY (SELECT 1) TO PROGRAM + 'rm -rf /tmp/datalake_fdw_conformance_file && mkdir -p /tmp/datalake_fdw_conformance_file'; + +\set prefix 'file:///tmp/datalake_fdw_conformance_file' +\set root '/tmp/datalake_fdw_conformance_file/' +\set volume NULL +\set kv NULL + +-- pg_regress feeds the script to psql on standard input, so there is no +-- script directory for \ir to resolve against; the path is relative to where +-- make runs, which is the module's own directory. +\i test/automation/sqlrepo/smoke/storage_conformance/sql/conformance_body.sql + +COPY (SELECT 1) TO PROGRAM 'rm -rf /tmp/datalake_fdw_conformance_file'; diff --git a/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_conformance/sql/conformance_s3.sql b/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_conformance/sql/conformance_s3.sql new file mode 100644 index 00000000000..b826b455e61 --- /dev/null +++ b/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_conformance/sql/conformance_s3.sql @@ -0,0 +1,67 @@ +-- The s3 backend, reached the way a user reaches it: through a volume server +-- and a user mapping, rather than by handing credentials to a function. +SET client_min_messages = warning; +CREATE EXTENSION IF NOT EXISTS datalake_fdw; +CREATE EXTENSION IF NOT EXISTS datalake_fdw_test; +RESET client_min_messages; + +SET log_statement = 'none'; +SET log_min_duration_statement = -1; +SET log_min_error_statement = 'panic'; + +-- Endpoint, bucket and credentials differ from one machine to the next, so +-- they are read with the echo off: what this file asserts must not depend on +-- where it ran. +\set ECHO none +-- An unset psql variable interpolates as its own name, so the optional ones +-- are given a value before \getenv has the chance to leave them undefined. +\set region '' +\set path_style '' +\getenv endpoint DATALAKE_TEST_S3_ENDPOINT +\getenv bucket DATALAKE_TEST_S3_BUCKET +\getenv access_key DATALAKE_TEST_S3_ACCESS_KEY +\getenv secret_key DATALAKE_TEST_S3_SECRET_KEY +\getenv region DATALAKE_TEST_S3_REGION +\getenv path_style DATALAKE_TEST_S3_PATH_STYLE + +SELECT to_char(clock_timestamp(), 'YYYYMMDDHH24MISSMS') || '_' || + pg_backend_pid() AS run \gset + +-- Server options take literals, so the values are assembled first and +-- interpolated as literals afterwards. +SELECT format('s3://%s/datalake_conformance/%s', :'bucket', :'run') AS base_path, + format('s3://%s/datalake_conformance/%s', :'bucket', :'run') AS prefix, + format('%s/datalake_conformance/%s/', :'bucket', :'run') AS root, + format('ARRAY[%L,%L,%L,%L,%L]', + 'endpoint=' || :'endpoint', 'region=' || :'region', + 'path_style_access=' || coalesce(nullif(:'path_style', ''), 'true'), + 'access_key_id=' || :'access_key', + 'secret_access_key=' || :'secret_key') AS kv, + coalesce(nullif(:'region', ''), 'us-east-1') AS server_region, + coalesce(nullif(:'path_style', ''), 'true') AS server_path_style \gset + +DROP SERVER IF EXISTS dlconf_volume CASCADE; +CREATE SERVER dlconf_volume FOREIGN DATA WRAPPER iceberg_volume_fdw OPTIONS ( + base_path :'base_path', + endpoint :'endpoint', + region :'server_region', + path_style_access :'server_path_style'); +CREATE USER MAPPING FOR CURRENT_USER SERVER dlconf_volume OPTIONS ( + access_key_id :'access_key', + secret_access_key :'secret_key'); + +\set volume '''dlconf_volume''' +\set ECHO all + +-- The volume exists and names this run's prefix. +SELECT count(*) = 1 AS volume_created +FROM pg_foreign_server WHERE srvname = 'dlconf_volume'; + +-- pg_regress feeds the script to psql on standard input, so there is no +-- script directory for \ir to resolve against; the path is relative to where +-- make runs, which is the module's own directory. +\i test/automation/sqlrepo/smoke/storage_conformance/sql/conformance_body.sql + +\set ECHO none +DROP SERVER dlconf_volume CASCADE; +\set ECHO all diff --git a/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_conformance/sql/volume_resolve.sql b/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_conformance/sql/volume_resolve.sql new file mode 100644 index 00000000000..6334cdb7493 --- /dev/null +++ b/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_conformance/sql/volume_resolve.sql @@ -0,0 +1,89 @@ +-- Resolving a volume: who may use it, whose credentials are used, and what +-- happens when a path does not belong to it. A file volume needs no service +-- to talk to, so this runs everywhere. +SET client_min_messages = warning; +CREATE EXTENSION IF NOT EXISTS datalake_fdw; +CREATE EXTENSION IF NOT EXISTS datalake_fdw_test; +RESET client_min_messages; + +COPY (SELECT 1) TO PROGRAM + 'rm -rf /tmp/datalake_fdw_volume && mkdir -p /tmp/datalake_fdw_volume/inside'; + +DROP SERVER IF EXISTS dlvol CASCADE; +DROP SERVER IF EXISTS dlvol_other CASCADE; +DROP ROLE IF EXISTS dlvol_user; +CREATE ROLE dlvol_user LOGIN; + +CREATE SERVER dlvol FOREIGN DATA WRAPPER iceberg_volume_fdw + OPTIONS (base_path 'file:///tmp/datalake_fdw_volume/inside'); +CREATE SERVER dlvol_other FOREIGN DATA WRAPPER iceberg_volume_fdw + OPTIONS (base_path 'file:///tmp/datalake_fdw_volume/elsewhere'); + +-- Through the volume, as its owner. +SELECT datalake_parquet_write('file:///tmp/datalake_fdw_volume/inside/a.parquet', + 'SELECT 42 AS id', 0, '', 'dlvol') AS rows_written; +SELECT id FROM datalake_parquet_read( + 'file:///tmp/datalake_fdw_volume/inside/a.parquet', 0, 0, '{}', 'dlvol') + AS t(id int); + +-- A path outside the volume is refused, even though the volume would have +-- been happy to lend its settings to it. +SELECT datalake_parquet_write('file:///tmp/datalake_fdw_volume/outside.parquet', + 'SELECT 1 AS id', 0, '', 'dlvol'); +-- And so is one under a different volume. +SELECT datalake_parquet_write('file:///tmp/datalake_fdw_volume/inside/b.parquet', + 'SELECT 1 AS id', 0, '', 'dlvol_other'); + +-- A volume that does not exist. +SELECT id FROM datalake_parquet_read( + 'file:///tmp/datalake_fdw_volume/inside/a.parquet', 0, 0, '{}', + 'no_such_volume') AS t(id int); + +-- Using a volume takes USAGE on it. +GRANT EXECUTE ON FUNCTION datalake_parquet_read(text, int, int, int[], text) + TO dlvol_user; +SET ROLE dlvol_user; +SELECT id FROM datalake_parquet_read( + 'file:///tmp/datalake_fdw_volume/inside/a.parquet', 0, 0, '{}', 'dlvol') + AS t(id int); +RESET ROLE; + +GRANT USAGE ON FOREIGN SERVER dlvol TO dlvol_user; +SET ROLE dlvol_user; +SELECT id FROM datalake_parquet_read( + 'file:///tmp/datalake_fdw_volume/inside/a.parquet', 0, 0, '{}', 'dlvol') + AS t(id int); +RESET ROLE; + +-- A PUBLIC mapping is what a user without one of their own gets. The file +-- backend ignores credentials, so what is asserted is that resolution finds +-- the mapping and still reaches the file, not that the values did anything. +CREATE USER MAPPING FOR PUBLIC SERVER dlvol + OPTIONS (access_key_id 'public-key', secret_access_key 'public-secret-value'); +SET ROLE dlvol_user; +SELECT id FROM datalake_parquet_read( + 'file:///tmp/datalake_fdw_volume/inside/a.parquet', 0, 0, '{}', 'dlvol') + AS t(id int); +RESET ROLE; + +CREATE USER MAPPING FOR dlvol_user SERVER dlvol + OPTIONS (access_key_id 'user-key', secret_access_key 'user-secret-value'); +SET ROLE dlvol_user; +SELECT id FROM datalake_parquet_read( + 'file:///tmp/datalake_fdw_volume/inside/a.parquet', 0, 0, '{}', 'dlvol') + AS t(id int); +RESET ROLE; + +-- A base_path is quoted back when it is rejected, and a URI can carry a +-- password in its userinfo. Neither the message nor the detail may reproduce +-- one: if this case ever prints "hunter2", the rejection leaked a credential +-- into the server log. +CREATE SERVER dlvol_secret FOREIGN DATA WRAPPER iceberg_volume_fdw + OPTIONS (base_path 's3://reader:hunter2@bucket/prefix'); +CREATE SERVER dlvol_signed FOREIGN DATA WRAPPER iceberg_volume_fdw + OPTIONS (base_path 's3://bucket/prefix?X-Amz-Signature=deadbeef'); + +DROP SERVER dlvol CASCADE; +DROP SERVER dlvol_other CASCADE; +DROP ROLE dlvol_user; +COPY (SELECT 1) TO PROGRAM 'rm -rf /tmp/datalake_fdw_volume'; diff --git a/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_local/expected/storage_local.out b/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_local/expected/storage_local.out new file mode 100644 index 00000000000..f45c9ae82ee --- /dev/null +++ b/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_local/expected/storage_local.out @@ -0,0 +1,128 @@ +-- Storage facade behavior shared by the built-in file backend and a backend +-- registered through the public plugin contract. +SET client_min_messages = warning; +CREATE EXTENSION IF NOT EXISTS datalake_fdw; +CREATE EXTENSION IF NOT EXISTS datalake_fdw_test; +RESET client_min_messages; +COPY (SELECT 1) TO PROGRAM + 'rm -rf /tmp/datalake_fdw_storage_local && mkdir -p /tmp/datalake_fdw_storage_local/file /tmp/datalake_fdw_storage_local/dltest'; +SELECT datalake_storage_write_text( + 'file:///tmp/datalake_fdw_storage_local/file/a.txt', 'file-data'); + datalake_storage_write_text +----------------------------- + 9 +(1 row) + +SELECT datalake_storage_read_text( + 'file:///tmp/datalake_fdw_storage_local/file/a.txt'); + datalake_storage_read_text +---------------------------- + file-data +(1 row) + +SELECT * FROM datalake_storage_list( + 'file:///tmp/datalake_fdw_storage_local/file'); + datalake_storage_list +-------------------------------------------- + /tmp/datalake_fdw_storage_local/file/a.txt +(1 row) + +\set VERBOSITY sqlstate +SELECT datalake_storage_write_text( + 'file:///tmp/datalake_fdw_storage_local/file/a.txt', 'replacement'); +ERROR: 42P07 +SELECT datalake_storage_read_text( + 'file:///tmp/datalake_fdw_storage_local/file/missing.txt'); +ERROR: 42704 +SELECT datalake_storage_read_text('file://host/tmp/a.txt'); +ERROR: 22023 +SELECT datalake_storage_read_text('/tmp/a.txt'); +ERROR: 22023 +-- A path may not climb out of the volume it was resolved against. +SELECT datalake_storage_read_text('file:///tmp/datalake_fdw_storage_local/..'); +ERROR: 22023 +\set VERBOSITY default +SELECT datalake_storage_read_text( + 'file:///tmp/datalake_fdw_storage_local/file/a.txt'); + datalake_storage_read_text +---------------------------- + file-data +(1 row) + +SELECT datalake_storage_write_text( + 'dltest:///tmp/datalake_fdw_storage_local/dltest/a.txt', 'dltest-data'); + datalake_storage_write_text +----------------------------- + 11 +(1 row) + +SELECT datalake_storage_read_text( + 'dltest:///tmp/datalake_fdw_storage_local/dltest/a.txt'); + datalake_storage_read_text +---------------------------- + dltest-data +(1 row) + +SELECT * FROM datalake_storage_list( + 'dltest:///tmp/datalake_fdw_storage_local/dltest'); + datalake_storage_list +----------------------- + a.txt +(1 row) + +\set VERBOSITY sqlstate +SELECT datalake_storage_write_text( + 'dltest:///tmp/datalake_fdw_storage_local/dltest/a.txt', 'replacement'); +ERROR: 42P07 +SELECT datalake_storage_read_text( + 'dltest:///tmp/datalake_fdw_storage_local/dltest/missing.txt'); +ERROR: 42704 +\set VERBOSITY default +SELECT datalake_storage_read_text( + 'dltest:///tmp/datalake_fdw_storage_local/dltest/a.txt'); + datalake_storage_read_text +---------------------------- + dltest-data +(1 row) + +-- A write that is refused leaves the file it refused to replace untouched, +-- and leaves nothing else behind either. +SELECT * FROM datalake_storage_list( + 'file:///tmp/datalake_fdw_storage_local/file'); + datalake_storage_list +-------------------------------------------- + /tmp/datalake_fdw_storage_local/file/a.txt +(1 row) + +SELECT datalake_storage_probe('s3'); + datalake_storage_probe +------------------------ + supported +(1 row) + +-- Each malformed registration must be rejected by the check it breaks, not by +-- some later one: every pattern names the field and the value it reports, so +-- a check that stopped working could not fall through to another and still +-- match. +SELECT kind, + datalake_storage_register_bad(kind) LIKE pattern AS rejected_by_its_check +FROM (VALUES + ('abi_version', '%ABI version mismatch: expected 1, got 2%'), + ('struct_size', '%struct size mismatch: expected at least %, got 0%'), + ('arrow_version', '%Arrow version mismatch: expected "%", got "0.0.0-test"%'), + ('abi_fingerprint', + '%ABI fingerprint mismatch: expected "%", got "gcc0;cxx11abi=9;arrow=0.0.0-test"%'), + ('duplicate', '%scheme expected to be unique, got duplicate "dltest"%') +) AS t(kind, pattern) +ORDER BY kind; + kind | rejected_by_its_check +-----------------+----------------------- + abi_fingerprint | t + abi_version | t + arrow_version | t + duplicate | t + struct_size | t +(5 rows) + +COPY (SELECT 1) TO PROGRAM 'rm -rf /tmp/datalake_fdw_storage_local'; +-- End of storage_local. diff --git a/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_local/expected/storage_local_1.out b/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_local/expected/storage_local_1.out new file mode 100644 index 00000000000..398a4ef06f4 --- /dev/null +++ b/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_local/expected/storage_local_1.out @@ -0,0 +1,128 @@ +-- Storage facade behavior shared by the built-in file backend and a backend +-- registered through the public plugin contract. +SET client_min_messages = warning; +CREATE EXTENSION IF NOT EXISTS datalake_fdw; +CREATE EXTENSION IF NOT EXISTS datalake_fdw_test; +RESET client_min_messages; +COPY (SELECT 1) TO PROGRAM + 'rm -rf /tmp/datalake_fdw_storage_local && mkdir -p /tmp/datalake_fdw_storage_local/file /tmp/datalake_fdw_storage_local/dltest'; +SELECT datalake_storage_write_text( + 'file:///tmp/datalake_fdw_storage_local/file/a.txt', 'file-data'); + datalake_storage_write_text +----------------------------- + 9 +(1 row) + +SELECT datalake_storage_read_text( + 'file:///tmp/datalake_fdw_storage_local/file/a.txt'); + datalake_storage_read_text +---------------------------- + file-data +(1 row) + +SELECT * FROM datalake_storage_list( + 'file:///tmp/datalake_fdw_storage_local/file'); + datalake_storage_list +-------------------------------------------- + /tmp/datalake_fdw_storage_local/file/a.txt +(1 row) + +\set VERBOSITY sqlstate +SELECT datalake_storage_write_text( + 'file:///tmp/datalake_fdw_storage_local/file/a.txt', 'replacement'); +ERROR: 42P07 +SELECT datalake_storage_read_text( + 'file:///tmp/datalake_fdw_storage_local/file/missing.txt'); +ERROR: 42704 +SELECT datalake_storage_read_text('file://host/tmp/a.txt'); +ERROR: 22023 +SELECT datalake_storage_read_text('/tmp/a.txt'); +ERROR: 22023 +-- A path may not climb out of the volume it was resolved against. +SELECT datalake_storage_read_text('file:///tmp/datalake_fdw_storage_local/..'); +ERROR: 22023 +\set VERBOSITY default +SELECT datalake_storage_read_text( + 'file:///tmp/datalake_fdw_storage_local/file/a.txt'); + datalake_storage_read_text +---------------------------- + file-data +(1 row) + +SELECT datalake_storage_write_text( + 'dltest:///tmp/datalake_fdw_storage_local/dltest/a.txt', 'dltest-data'); + datalake_storage_write_text +----------------------------- + 11 +(1 row) + +SELECT datalake_storage_read_text( + 'dltest:///tmp/datalake_fdw_storage_local/dltest/a.txt'); + datalake_storage_read_text +---------------------------- + dltest-data +(1 row) + +SELECT * FROM datalake_storage_list( + 'dltest:///tmp/datalake_fdw_storage_local/dltest'); + datalake_storage_list +----------------------- + a.txt +(1 row) + +\set VERBOSITY sqlstate +SELECT datalake_storage_write_text( + 'dltest:///tmp/datalake_fdw_storage_local/dltest/a.txt', 'replacement'); +ERROR: 42P07 +SELECT datalake_storage_read_text( + 'dltest:///tmp/datalake_fdw_storage_local/dltest/missing.txt'); +ERROR: 42704 +\set VERBOSITY default +SELECT datalake_storage_read_text( + 'dltest:///tmp/datalake_fdw_storage_local/dltest/a.txt'); + datalake_storage_read_text +---------------------------- + dltest-data +(1 row) + +-- A write that is refused leaves the file it refused to replace untouched, +-- and leaves nothing else behind either. +SELECT * FROM datalake_storage_list( + 'file:///tmp/datalake_fdw_storage_local/file'); + datalake_storage_list +-------------------------------------------- + /tmp/datalake_fdw_storage_local/file/a.txt +(1 row) + +SELECT datalake_storage_probe('s3'); + datalake_storage_probe +-------------------------------------------------------------------------------------------------------------------------------------------------------------------------- + datalake_fdw was built without the AWS SDK for C++, so s3:// locations cannot be opened; rebuild the extension with AWS_SDK_PREFIX= pointing at an installed SDK +(1 row) + +-- Each malformed registration must be rejected by the check it breaks, not by +-- some later one: every pattern names the field and the value it reports, so +-- a check that stopped working could not fall through to another and still +-- match. +SELECT kind, + datalake_storage_register_bad(kind) LIKE pattern AS rejected_by_its_check +FROM (VALUES + ('abi_version', '%ABI version mismatch: expected 1, got 2%'), + ('struct_size', '%struct size mismatch: expected at least %, got 0%'), + ('arrow_version', '%Arrow version mismatch: expected "%", got "0.0.0-test"%'), + ('abi_fingerprint', + '%ABI fingerprint mismatch: expected "%", got "gcc0;cxx11abi=9;arrow=0.0.0-test"%'), + ('duplicate', '%scheme expected to be unique, got duplicate "dltest"%') +) AS t(kind, pattern) +ORDER BY kind; + kind | rejected_by_its_check +-----------------+----------------------- + abi_fingerprint | t + abi_version | t + arrow_version | t + duplicate | t + struct_size | t +(5 rows) + +COPY (SELECT 1) TO PROGRAM 'rm -rf /tmp/datalake_fdw_storage_local'; +-- End of storage_local. diff --git a/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_local/sql/storage_local.sql b/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_local/sql/storage_local.sql new file mode 100644 index 00000000000..6ab78953e4b --- /dev/null +++ b/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_local/sql/storage_local.sql @@ -0,0 +1,71 @@ +-- Storage facade behavior shared by the built-in file backend and a backend +-- registered through the public plugin contract. +SET client_min_messages = warning; +CREATE EXTENSION IF NOT EXISTS datalake_fdw; +CREATE EXTENSION IF NOT EXISTS datalake_fdw_test; +RESET client_min_messages; + +COPY (SELECT 1) TO PROGRAM + 'rm -rf /tmp/datalake_fdw_storage_local && mkdir -p /tmp/datalake_fdw_storage_local/file /tmp/datalake_fdw_storage_local/dltest'; + +SELECT datalake_storage_write_text( + 'file:///tmp/datalake_fdw_storage_local/file/a.txt', 'file-data'); +SELECT datalake_storage_read_text( + 'file:///tmp/datalake_fdw_storage_local/file/a.txt'); +SELECT * FROM datalake_storage_list( + 'file:///tmp/datalake_fdw_storage_local/file'); + +\set VERBOSITY sqlstate +SELECT datalake_storage_write_text( + 'file:///tmp/datalake_fdw_storage_local/file/a.txt', 'replacement'); +SELECT datalake_storage_read_text( + 'file:///tmp/datalake_fdw_storage_local/file/missing.txt'); +SELECT datalake_storage_read_text('file://host/tmp/a.txt'); +SELECT datalake_storage_read_text('/tmp/a.txt'); +-- A path may not climb out of the volume it was resolved against. +SELECT datalake_storage_read_text('file:///tmp/datalake_fdw_storage_local/..'); +\set VERBOSITY default +SELECT datalake_storage_read_text( + 'file:///tmp/datalake_fdw_storage_local/file/a.txt'); + +SELECT datalake_storage_write_text( + 'dltest:///tmp/datalake_fdw_storage_local/dltest/a.txt', 'dltest-data'); +SELECT datalake_storage_read_text( + 'dltest:///tmp/datalake_fdw_storage_local/dltest/a.txt'); +SELECT * FROM datalake_storage_list( + 'dltest:///tmp/datalake_fdw_storage_local/dltest'); +\set VERBOSITY sqlstate +SELECT datalake_storage_write_text( + 'dltest:///tmp/datalake_fdw_storage_local/dltest/a.txt', 'replacement'); +SELECT datalake_storage_read_text( + 'dltest:///tmp/datalake_fdw_storage_local/dltest/missing.txt'); +\set VERBOSITY default +SELECT datalake_storage_read_text( + 'dltest:///tmp/datalake_fdw_storage_local/dltest/a.txt'); + +-- A write that is refused leaves the file it refused to replace untouched, +-- and leaves nothing else behind either. +SELECT * FROM datalake_storage_list( + 'file:///tmp/datalake_fdw_storage_local/file'); + +SELECT datalake_storage_probe('s3'); + +-- Each malformed registration must be rejected by the check it breaks, not by +-- some later one: every pattern names the field and the value it reports, so +-- a check that stopped working could not fall through to another and still +-- match. +SELECT kind, + datalake_storage_register_bad(kind) LIKE pattern AS rejected_by_its_check +FROM (VALUES + ('abi_version', '%ABI version mismatch: expected 1, got 2%'), + ('struct_size', '%struct size mismatch: expected at least %, got 0%'), + ('arrow_version', '%Arrow version mismatch: expected "%", got "0.0.0-test"%'), + ('abi_fingerprint', + '%ABI fingerprint mismatch: expected "%", got "gcc0;cxx11abi=9;arrow=0.0.0-test"%'), + ('duplicate', '%scheme expected to be unique, got duplicate "dltest"%') +) AS t(kind, pattern) +ORDER BY kind; + +COPY (SELECT 1) TO PROGRAM 'rm -rf /tmp/datalake_fdw_storage_local'; + +-- End of storage_local. diff --git a/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_s3/expected/storage_s3.out b/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_s3/expected/storage_s3.out new file mode 100644 index 00000000000..dd116f04d20 --- /dev/null +++ b/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_s3/expected/storage_s3.out @@ -0,0 +1,214 @@ +-- The s3 backend against a real S3-compatible service. Endpoint and +-- credentials come from the environment, so this category only runs where +-- DATALAKE_TEST_S3_ENDPOINT is set; the Makefile skips it otherwise. +SET client_min_messages = warning; +CREATE EXTENSION IF NOT EXISTS datalake_fdw; +CREATE EXTENSION IF NOT EXISTS datalake_fdw_test; +RESET client_min_messages; +-- This file hands the server credentials, and psql puts them in the statement +-- it sends, so with statement logging on they would land in the server log +-- and no check could tell that apart from the extension leaking them. With +-- it off, a credential in the log came from the code under test. +SET log_statement = 'none'; +SET log_min_duration_statement = -1; +SET log_min_error_statement = 'panic'; +-- Which service, which bucket and whose credentials differ from one machine +-- to the next, so they are read into settings with the echo off: what this +-- file asserts must not depend on where it ran. Objects also go under a +-- prefix of this run's own, so two runs against one bucket cannot collide. +\set ECHO none +SELECT current_setting('datalake.s3_endpoint') <> '' AS have_endpoint, + current_setting('datalake.s3_bucket') <> '' AS have_bucket, + current_setting('datalake.s3_run') <> '' AS have_run; + have_endpoint | have_bucket | have_run +---------------+-------------+---------- + t | t | t +(1 row) + +CREATE FUNCTION s3_kv(secret text DEFAULT NULL) RETURNS text[] LANGUAGE sql AS $$ + SELECT ARRAY['endpoint=' || current_setting('datalake.s3_endpoint'), + 'region=' || current_setting('datalake.s3_region'), + 'path_style_access=' || current_setting('datalake.s3_path_style'), + 'access_key_id=' || current_setting('datalake.s3_access_key'), + 'secret_access_key=' || + coalesce(secret, current_setting('datalake.s3_secret'))] +$$; +CREATE FUNCTION s3_prefix() RETURNS text LANGUAGE sql AS $$ + SELECT format('s3://%s/datalake_regress/%s', + current_setting('datalake.s3_bucket'), + current_setting('datalake.s3_run')) +$$; +CREATE FUNCTION s3_uri(name text) RETURNS text LANGUAGE sql AS $$ + SELECT s3_prefix() || '/' || name +$$; +CREATE FUNCTION s3_error(uri text, kv text[]) RETURNS text LANGUAGE plpgsql AS $fn$ +DECLARE + message text; + detail text; +BEGIN + PERFORM datalake_storage_read_text(uri, kv); + RETURN 'no error'; +EXCEPTION WHEN OTHERS THEN + GET STACKED DIAGNOSTICS message = MESSAGE_TEXT, detail = PG_EXCEPTION_DETAIL; + RETURN message || ' | ' || detail; +END +$fn$; +-- Round trip. +SELECT datalake_storage_write_text(s3_uri('a.txt'), 'first-object', s3_kv()); + datalake_storage_write_text +----------------------------- + 12 +(1 row) + +SELECT datalake_storage_read_text(s3_uri('a.txt'), s3_kv()); + datalake_storage_read_text +---------------------------- + first-object +(1 row) + +-- Larger than the 8 MiB part size, so this goes out as a multipart upload and +-- has to come back byte for byte. +SELECT datalake_storage_write_text(s3_uri('big.bin'), + repeat('0123456789', 900000), s3_kv()) AS bytes_written; + bytes_written +--------------- + 9000000 +(1 row) + +SELECT length(datalake_storage_read_text(s3_uri('big.bin'), s3_kv())) AS bytes_read, + md5(datalake_storage_read_text(s3_uri('big.bin'), s3_kv())) + = md5(repeat('0123456789', 900000)) AS same_bytes; + bytes_read | same_bytes +------------+------------ + 9000000 | t +(1 row) + +-- Listing reports this run's objects by their native path. +SELECT replace(path, + format('%s/datalake_regress/%s/', current_setting('datalake.s3_bucket'), + current_setting('datalake.s3_run')), '') AS object +FROM datalake_storage_list(s3_prefix(), s3_kv()) AS path +ORDER BY 1; + object +--------- + a.txt + big.bin +(2 rows) + +\set VERBOSITY sqlstate +-- An object that exists is never replaced. +SELECT datalake_storage_write_text(s3_uri('a.txt'), 'replacement', s3_kv()); +ERROR: 42P07 +-- A key that is not there, and a bucket that is not there. +SELECT datalake_storage_read_text(s3_uri('missing.txt'), s3_kv()); +ERROR: 42704 +SELECT datalake_storage_read_text('s3://datalake-no-such-bucket-9f2b/x.txt', s3_kv()); +ERROR: 42704 +-- A wrong secret is refused. +SELECT datalake_storage_read_text(s3_uri('a.txt'), + s3_kv(current_setting('datalake.s3_bad_secret'))); +ERROR: 58030 +\set VERBOSITY default +-- The refused write left the object as it was. +SELECT datalake_storage_read_text(s3_uri('a.txt'), s3_kv()); + datalake_storage_read_text +---------------------------- + first-object +(1 row) + +-- What the user is told names the object but not the secret. The text of an +-- SDK message varies between services, so this asks what must and must not be +-- in it rather than pinning the whole string. +SELECT s3_error(s3_uri('a.txt'), s3_kv(current_setting('datalake.s3_bad_secret'))) + LIKE '%' || current_setting('datalake.s3_bad_secret') || '%' + AS leaks_the_secret, + s3_error(s3_uri('a.txt'), s3_kv(current_setting('datalake.s3_bad_secret'))) + LIKE '%a.txt%' AS names_the_object, + s3_error(s3_uri('a.txt'), s3_kv()) + = 'no error' AS good_credentials_still_work; + leaks_the_secret | names_the_object | good_credentials_still_work +------------------+------------------+----------------------------- + f | t | t +(1 row) + +-- Redaction, tested where the service really does echo the value back: a +-- bucket name appears in the error about it, so a run whose secret IS that +-- bucket name must come back with the name masked. Remove the redaction and +-- this fails, which the wrong-secret case above cannot claim. +SELECT s3_error('s3://dl-redaction-probe-9f2b/x.txt', + ARRAY['endpoint=' || current_setting('datalake.s3_endpoint'), + 'region=' || current_setting('datalake.s3_region'), + 'path_style_access=' || current_setting('datalake.s3_path_style'), + 'access_key_id=' || current_setting('datalake.s3_access_key'), + 'secret_access_key=dl-redaction-probe-9f2b']) + LIKE '%dl-redaction-probe-9f2b%' AS leaks_the_secret, + s3_error('s3://dl-redaction-probe-9f2b/x.txt', + ARRAY['endpoint=' || current_setting('datalake.s3_endpoint'), + 'region=' || current_setting('datalake.s3_region'), + 'path_style_access=' || current_setting('datalake.s3_path_style'), + 'access_key_id=' || current_setting('datalake.s3_access_key'), + 'secret_access_key=dl-redaction-probe-9f2b']) + LIKE '%***%' AS masked_it; + leaks_the_secret | masked_it +------------------+----------- + f | t +(1 row) + +-- Half a credential pair is a mistake, not a reason to fall back to whatever +-- identity the host happens to have. +\set VERBOSITY sqlstate +SELECT datalake_storage_read_text(s3_uri('a.txt'), + ARRAY['endpoint=' || current_setting('datalake.s3_endpoint'), + 'region=us-east-1', 'path_style_access=true', + 'access_key_id=' || current_setting('datalake.s3_access_key')]); +ERROR: 22023 +\set VERBOSITY default +-- Asking for host-style addressing against an endpoint that is an IP address +-- still works, because a bucket name cannot be prepended to an IP and the SDK +-- falls back to path style. Worth pinning: it is the reason a wrong +-- path_style_access setting does not fail loudly in a lab. +SELECT datalake_storage_read_text(s3_uri('a.txt'), + ARRAY['endpoint=' || current_setting('datalake.s3_endpoint'), + 'region=us-east-1', 'path_style_access=false', + 'access_key_id=' || current_setting('datalake.s3_access_key'), + 'secret_access_key=' || current_setting('datalake.s3_secret')]) + AS host_style_against_an_ip; + host_style_against_an_ip +-------------------------- + first-object +(1 row) + +-- An endpoint that answers nothing has to become an error while someone is +-- still waiting for it, rather than a session that cannot be cancelled. +SELECT s3_error(s3_uri('a.txt'), + ARRAY['endpoint=http://10.255.255.1:9000', + 'region=us-east-1', 'path_style_access=true', + 'access_key_id=x', 'secret_access_key=y']) + <> 'no error' AS blackhole_reported, + clock_timestamp() - statement_timestamp() < interval '30 seconds' + AS within_the_bound; + blackhole_reported | within_the_bound +--------------------+------------------ + t | t +(1 row) + +-- The session still works afterwards. +SELECT datalake_storage_read_text(s3_uri('a.txt'), s3_kv()); + datalake_storage_read_text +---------------------------- + first-object +(1 row) + +-- Take this run's objects back out again. +SELECT datalake_storage_delete(s3_uri('a.txt'), s3_kv()) AS deleted_a, + datalake_storage_delete(s3_uri('big.bin'), s3_kv()) AS deleted_big; + deleted_a | deleted_big +-----------+------------- + t | t +(1 row) + +\set VERBOSITY sqlstate +SELECT datalake_storage_delete(s3_uri('a.txt'), s3_kv()); +ERROR: 42704 +\set VERBOSITY default +DROP FUNCTION s3_error(text, text[]), s3_uri(text), s3_prefix(), s3_kv(text); diff --git a/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_s3/expected/storage_s3_pagination.out b/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_s3/expected/storage_s3_pagination.out new file mode 100644 index 00000000000..315a0de825f --- /dev/null +++ b/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_s3/expected/storage_s3_pagination.out @@ -0,0 +1,58 @@ +-- Listing more objects than one page holds. Roughly four thousand requests +-- go over the wire here, which is minutes rather than seconds, so this is its +-- own case and runs where DATALAKE_TEST_S3_PAGINATION says to -- CI, not every +-- local build. +SET client_min_messages = warning; +CREATE EXTENSION IF NOT EXISTS datalake_fdw; +CREATE EXTENSION IF NOT EXISTS datalake_fdw_test; +RESET client_min_messages; +SET log_statement = 'none'; +SET log_min_duration_statement = -1; +SET log_min_error_statement = 'panic'; +\set ECHO none +CREATE FUNCTION s3_kv() RETURNS text[] LANGUAGE sql AS $$ + SELECT ARRAY['endpoint=' || current_setting('datalake.s3_endpoint'), + 'region=' || current_setting('datalake.s3_region'), + 'path_style_access=' || current_setting('datalake.s3_path_style'), + 'access_key_id=' || current_setting('datalake.s3_access_key'), + 'secret_access_key=' || current_setting('datalake.s3_secret')] +$$; +CREATE FUNCTION s3_uri(name text) RETURNS text LANGUAGE sql AS $$ + SELECT format('s3://%s/datalake_regress/%s/%s', + current_setting('datalake.s3_bucket'), + current_setting('datalake.s3_run'), name) +$$; +CREATE FUNCTION s3_prefix() RETURNS text LANGUAGE sql AS $$ + SELECT format('s3://%s/datalake_regress/%s', + current_setting('datalake.s3_bucket'), + current_setting('datalake.s3_run')) +$$; +-- More objects than one listing page holds. Each aggregate reads the value +-- the function returned rather than counting rows: a target list nothing +-- refers to can be optimised away, and then the writes never happen while the +-- count still looks right. +SELECT count(*) FILTER (WHERE bytes > 0) AS created FROM ( + SELECT datalake_storage_write_text(s3_uri('page/' || lpad(i::text, 5, '0')), + i::text, s3_kv()) AS bytes + FROM generate_series(1, 1100) i) AS w; + created +--------- + 1100 +(1 row) + +SELECT count(*) AS listed FROM datalake_storage_list(s3_prefix() || '/page', s3_kv()); + listed +-------- + 1100 +(1 row) + +SELECT count(*) FILTER (WHERE gone) AS deleted FROM ( + SELECT datalake_storage_delete(s3_uri('page/' || lpad(i::text, 5, '0')), + s3_kv()) AS gone + FROM generate_series(1, 1100) i) AS d; + deleted +--------- + 1100 +(1 row) + +DROP FUNCTION s3_prefix(), s3_uri(text), s3_kv(); diff --git a/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_s3/sql/storage_s3.sql b/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_s3/sql/storage_s3.sql new file mode 100644 index 00000000000..bdec675cb9b --- /dev/null +++ b/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_s3/sql/storage_s3.sql @@ -0,0 +1,194 @@ +-- The s3 backend against a real S3-compatible service. Endpoint and +-- credentials come from the environment, so this category only runs where +-- DATALAKE_TEST_S3_ENDPOINT is set; the Makefile skips it otherwise. +SET client_min_messages = warning; +CREATE EXTENSION IF NOT EXISTS datalake_fdw; +CREATE EXTENSION IF NOT EXISTS datalake_fdw_test; +RESET client_min_messages; + +-- This file hands the server credentials, and psql puts them in the statement +-- it sends, so with statement logging on they would land in the server log +-- and no check could tell that apart from the extension leaking them. With +-- it off, a credential in the log came from the code under test. +SET log_statement = 'none'; +SET log_min_duration_statement = -1; +SET log_min_error_statement = 'panic'; + +-- Which service, which bucket and whose credentials differ from one machine +-- to the next, so they are read into settings with the echo off: what this +-- file asserts must not depend on where it ran. Objects also go under a +-- prefix of this run's own, so two runs against one bucket cannot collide. +\set ECHO none +-- \getenv leaves the variable unset when the environment does not define it, +-- and an unset variable interpolates as its own name, which is a syntax error +-- rather than a default. So the optional ones are given values first. +\set region '' +\set path_style '' +\set bad_secret '' +\getenv endpoint DATALAKE_TEST_S3_ENDPOINT +\getenv bucket DATALAKE_TEST_S3_BUCKET +\getenv access_key DATALAKE_TEST_S3_ACCESS_KEY +\getenv secret_key DATALAKE_TEST_S3_SECRET_KEY +\getenv region DATALAKE_TEST_S3_REGION +\getenv path_style DATALAKE_TEST_S3_PATH_STYLE +\getenv bad_secret DATALAKE_TEST_S3_BAD_SECRET + +SELECT set_config('datalake.s3_endpoint', :'endpoint', false) AS endpoint, + set_config('datalake.s3_bucket', :'bucket', false) AS bucket, + set_config('datalake.s3_access_key', :'access_key', false) AS access_key, + set_config('datalake.s3_secret', :'secret_key', false) AS secret, + set_config('datalake.s3_region', + coalesce(nullif(:'region', ''), 'us-east-1'), false) AS region, + set_config('datalake.s3_path_style', + coalesce(nullif(:'path_style', ''), 'true'), false) AS path_style, + set_config('datalake.s3_run', + to_char(clock_timestamp(), 'YYYYMMDDHH24MISSMS') || '_' || + pg_backend_pid(), false) AS run, + -- A wrong secret, taken from the environment rather than written here: + -- whatever this file says ends up in the server log as statement text, + -- and a secret in a log is the very thing the harness watches for. + set_config('datalake.s3_bad_secret', + coalesce(nullif(:'bad_secret', ''), 'not-the-secret'), + false) AS bad_secret +\gset +\set ECHO all + +SELECT current_setting('datalake.s3_endpoint') <> '' AS have_endpoint, + current_setting('datalake.s3_bucket') <> '' AS have_bucket, + current_setting('datalake.s3_run') <> '' AS have_run; + +CREATE FUNCTION s3_kv(secret text DEFAULT NULL) RETURNS text[] LANGUAGE sql AS $$ + SELECT ARRAY['endpoint=' || current_setting('datalake.s3_endpoint'), + 'region=' || current_setting('datalake.s3_region'), + 'path_style_access=' || current_setting('datalake.s3_path_style'), + 'access_key_id=' || current_setting('datalake.s3_access_key'), + 'secret_access_key=' || + coalesce(secret, current_setting('datalake.s3_secret'))] +$$; +CREATE FUNCTION s3_prefix() RETURNS text LANGUAGE sql AS $$ + SELECT format('s3://%s/datalake_regress/%s', + current_setting('datalake.s3_bucket'), + current_setting('datalake.s3_run')) +$$; +CREATE FUNCTION s3_uri(name text) RETURNS text LANGUAGE sql AS $$ + SELECT s3_prefix() || '/' || name +$$; + +CREATE FUNCTION s3_error(uri text, kv text[]) RETURNS text LANGUAGE plpgsql AS $fn$ +DECLARE + message text; + detail text; +BEGIN + PERFORM datalake_storage_read_text(uri, kv); + RETURN 'no error'; +EXCEPTION WHEN OTHERS THEN + GET STACKED DIAGNOSTICS message = MESSAGE_TEXT, detail = PG_EXCEPTION_DETAIL; + RETURN message || ' | ' || detail; +END +$fn$; + +-- Round trip. +SELECT datalake_storage_write_text(s3_uri('a.txt'), 'first-object', s3_kv()); +SELECT datalake_storage_read_text(s3_uri('a.txt'), s3_kv()); + +-- Larger than the 8 MiB part size, so this goes out as a multipart upload and +-- has to come back byte for byte. +SELECT datalake_storage_write_text(s3_uri('big.bin'), + repeat('0123456789', 900000), s3_kv()) AS bytes_written; +SELECT length(datalake_storage_read_text(s3_uri('big.bin'), s3_kv())) AS bytes_read, + md5(datalake_storage_read_text(s3_uri('big.bin'), s3_kv())) + = md5(repeat('0123456789', 900000)) AS same_bytes; + +-- Listing reports this run's objects by their native path. +SELECT replace(path, + format('%s/datalake_regress/%s/', current_setting('datalake.s3_bucket'), + current_setting('datalake.s3_run')), '') AS object +FROM datalake_storage_list(s3_prefix(), s3_kv()) AS path +ORDER BY 1; + +\set VERBOSITY sqlstate +-- An object that exists is never replaced. +SELECT datalake_storage_write_text(s3_uri('a.txt'), 'replacement', s3_kv()); +-- A key that is not there, and a bucket that is not there. +SELECT datalake_storage_read_text(s3_uri('missing.txt'), s3_kv()); +SELECT datalake_storage_read_text('s3://datalake-no-such-bucket-9f2b/x.txt', s3_kv()); +-- A wrong secret is refused. +SELECT datalake_storage_read_text(s3_uri('a.txt'), + s3_kv(current_setting('datalake.s3_bad_secret'))); +\set VERBOSITY default + +-- The refused write left the object as it was. +SELECT datalake_storage_read_text(s3_uri('a.txt'), s3_kv()); + +-- What the user is told names the object but not the secret. The text of an +-- SDK message varies between services, so this asks what must and must not be +-- in it rather than pinning the whole string. + +SELECT s3_error(s3_uri('a.txt'), s3_kv(current_setting('datalake.s3_bad_secret'))) + LIKE '%' || current_setting('datalake.s3_bad_secret') || '%' + AS leaks_the_secret, + s3_error(s3_uri('a.txt'), s3_kv(current_setting('datalake.s3_bad_secret'))) + LIKE '%a.txt%' AS names_the_object, + s3_error(s3_uri('a.txt'), s3_kv()) + = 'no error' AS good_credentials_still_work; + +-- Redaction, tested where the service really does echo the value back: a +-- bucket name appears in the error about it, so a run whose secret IS that +-- bucket name must come back with the name masked. Remove the redaction and +-- this fails, which the wrong-secret case above cannot claim. +SELECT s3_error('s3://dl-redaction-probe-9f2b/x.txt', + ARRAY['endpoint=' || current_setting('datalake.s3_endpoint'), + 'region=' || current_setting('datalake.s3_region'), + 'path_style_access=' || current_setting('datalake.s3_path_style'), + 'access_key_id=' || current_setting('datalake.s3_access_key'), + 'secret_access_key=dl-redaction-probe-9f2b']) + LIKE '%dl-redaction-probe-9f2b%' AS leaks_the_secret, + s3_error('s3://dl-redaction-probe-9f2b/x.txt', + ARRAY['endpoint=' || current_setting('datalake.s3_endpoint'), + 'region=' || current_setting('datalake.s3_region'), + 'path_style_access=' || current_setting('datalake.s3_path_style'), + 'access_key_id=' || current_setting('datalake.s3_access_key'), + 'secret_access_key=dl-redaction-probe-9f2b']) + LIKE '%***%' AS masked_it; + +-- Half a credential pair is a mistake, not a reason to fall back to whatever +-- identity the host happens to have. +\set VERBOSITY sqlstate +SELECT datalake_storage_read_text(s3_uri('a.txt'), + ARRAY['endpoint=' || current_setting('datalake.s3_endpoint'), + 'region=us-east-1', 'path_style_access=true', + 'access_key_id=' || current_setting('datalake.s3_access_key')]); +\set VERBOSITY default + +-- Asking for host-style addressing against an endpoint that is an IP address +-- still works, because a bucket name cannot be prepended to an IP and the SDK +-- falls back to path style. Worth pinning: it is the reason a wrong +-- path_style_access setting does not fail loudly in a lab. +SELECT datalake_storage_read_text(s3_uri('a.txt'), + ARRAY['endpoint=' || current_setting('datalake.s3_endpoint'), + 'region=us-east-1', 'path_style_access=false', + 'access_key_id=' || current_setting('datalake.s3_access_key'), + 'secret_access_key=' || current_setting('datalake.s3_secret')]) + AS host_style_against_an_ip; + +-- An endpoint that answers nothing has to become an error while someone is +-- still waiting for it, rather than a session that cannot be cancelled. +SELECT s3_error(s3_uri('a.txt'), + ARRAY['endpoint=http://10.255.255.1:9000', + 'region=us-east-1', 'path_style_access=true', + 'access_key_id=x', 'secret_access_key=y']) + <> 'no error' AS blackhole_reported, + clock_timestamp() - statement_timestamp() < interval '30 seconds' + AS within_the_bound; + +-- The session still works afterwards. +SELECT datalake_storage_read_text(s3_uri('a.txt'), s3_kv()); + +-- Take this run's objects back out again. +SELECT datalake_storage_delete(s3_uri('a.txt'), s3_kv()) AS deleted_a, + datalake_storage_delete(s3_uri('big.bin'), s3_kv()) AS deleted_big; +\set VERBOSITY sqlstate +SELECT datalake_storage_delete(s3_uri('a.txt'), s3_kv()); +\set VERBOSITY default + +DROP FUNCTION s3_error(text, text[]), s3_uri(text), s3_prefix(), s3_kv(text); diff --git a/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_s3/sql/storage_s3_pagination.sql b/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_s3/sql/storage_s3_pagination.sql new file mode 100644 index 00000000000..99b20ca4b23 --- /dev/null +++ b/contrib/datalake_fdw/test/automation/sqlrepo/smoke/storage_s3/sql/storage_s3_pagination.sql @@ -0,0 +1,70 @@ +-- Listing more objects than one page holds. Roughly four thousand requests +-- go over the wire here, which is minutes rather than seconds, so this is its +-- own case and runs where DATALAKE_TEST_S3_PAGINATION says to -- CI, not every +-- local build. +SET client_min_messages = warning; +CREATE EXTENSION IF NOT EXISTS datalake_fdw; +CREATE EXTENSION IF NOT EXISTS datalake_fdw_test; +RESET client_min_messages; + +SET log_statement = 'none'; +SET log_min_duration_statement = -1; +SET log_min_error_statement = 'panic'; + +\set ECHO none +\set region '' +\set path_style '' +\getenv endpoint DATALAKE_TEST_S3_ENDPOINT +\getenv bucket DATALAKE_TEST_S3_BUCKET +\getenv access_key DATALAKE_TEST_S3_ACCESS_KEY +\getenv secret_key DATALAKE_TEST_S3_SECRET_KEY +\getenv region DATALAKE_TEST_S3_REGION +\getenv path_style DATALAKE_TEST_S3_PATH_STYLE + +SELECT set_config('datalake.s3_endpoint', :'endpoint', false) AS endpoint, + set_config('datalake.s3_bucket', :'bucket', false) AS bucket, + set_config('datalake.s3_access_key', :'access_key', false) AS access_key, + set_config('datalake.s3_secret', :'secret_key', false) AS secret, + set_config('datalake.s3_region', + coalesce(nullif(:'region', ''), 'us-east-1'), false) AS region, + set_config('datalake.s3_path_style', + coalesce(nullif(:'path_style', ''), 'true'), false) AS path_style, + set_config('datalake.s3_run', + to_char(clock_timestamp(), 'YYYYMMDDHH24MISSMS') || '_' || + pg_backend_pid(), false) AS run +\gset +\set ECHO all + +CREATE FUNCTION s3_kv() RETURNS text[] LANGUAGE sql AS $$ + SELECT ARRAY['endpoint=' || current_setting('datalake.s3_endpoint'), + 'region=' || current_setting('datalake.s3_region'), + 'path_style_access=' || current_setting('datalake.s3_path_style'), + 'access_key_id=' || current_setting('datalake.s3_access_key'), + 'secret_access_key=' || current_setting('datalake.s3_secret')] +$$; +CREATE FUNCTION s3_uri(name text) RETURNS text LANGUAGE sql AS $$ + SELECT format('s3://%s/datalake_regress/%s/%s', + current_setting('datalake.s3_bucket'), + current_setting('datalake.s3_run'), name) +$$; +CREATE FUNCTION s3_prefix() RETURNS text LANGUAGE sql AS $$ + SELECT format('s3://%s/datalake_regress/%s', + current_setting('datalake.s3_bucket'), + current_setting('datalake.s3_run')) +$$; + +-- More objects than one listing page holds. Each aggregate reads the value +-- the function returned rather than counting rows: a target list nothing +-- refers to can be optimised away, and then the writes never happen while the +-- count still looks right. +SELECT count(*) FILTER (WHERE bytes > 0) AS created FROM ( + SELECT datalake_storage_write_text(s3_uri('page/' || lpad(i::text, 5, '0')), + i::text, s3_kv()) AS bytes + FROM generate_series(1, 1100) i) AS w; +SELECT count(*) AS listed FROM datalake_storage_list(s3_prefix() || '/page', s3_kv()); +SELECT count(*) FILTER (WHERE gone) AS deleted FROM ( + SELECT datalake_storage_delete(s3_uri('page/' || lpad(i::text, 5, '0')), + s3_kv()) AS gone + FROM generate_series(1, 1100) i) AS d; + +DROP FUNCTION s3_prefix(), s3_uri(text), s3_kv();