Skip to content

docs: explain S3 object-store configuration for SQL - #1716

Open
emecii wants to merge 2 commits into
apache:mainfrom
emecii:docs/s3-sql-object-store
Open

emecii wants to merge 2 commits into
apache:mainfrom
emecii:docs/s3-sql-object-store

Conversation

@emecii

@emecii emecii commented Sep 7, 2026

Copy link
Copy Markdown

Which issue does this PR close?

None. Related to #970, which remains open: this PR does not implement the requested aws.* SQL options.

Rationale for this change

The Python object-store guide demonstrates register_parquet but does not show how to create and query a SQL external table using the same registered store. This is a documentation improvement for that existing workflow.

What changes are included in this PR?

  • Show the imports, context setup, environment credential prerequisites, and bucket-specific object-store registration.
  • Demonstrate CREATE EXTERNAL TABLE and a SQL query using that store.
  • Add a network-free Python test that reads Parquet through an s3:// URL backed by a registered LocalFileSystem and checks the rows and non-null count.

Are there any user-facing changes?

Documentation only; no runtime API or SQL-option support changes.

Validation: 26 object-store tests pass, all 10 Python/code-cell snippets parse, applicable pre-commit hooks and git diff --check pass. Both S3 documentation snippets also read three rows and return a non-null count of 2 against a loopback Moto server, adapting only the endpoint and HTTP allowance and using dummy environment credentials. The original #970 SQL still fails with Could not find config namespace "aws", including after store registration.

Tests used an existing locally built 54.0.0 wheel from commit cee8a5d33db5dd7cf40f9a07cc6e2764c818db6c; its crates/core/src and python/datafusion sources match this branch. This is not a fresh wheel build from this PR head. No real AWS credentials or AWS SDK credential-chain behavior were validated.

AI-assisted documentation, test, and review follow-up. Open scope question: whether to retain this docs improvement or implement CLI-style AWS SQL-option processing in Python to address #970.

Signed-off-by: Yifan Chen <emecii23@gmail.com>

@timsaucer timsaucer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aside from the quick comments below, I don't think this PR actually addresses or demonstrates what the issue asked for.

Comment thread docs/source/user-guide/data-sources.md Outdated
Comment on lines +130 to +133
Configure S3 access on an {py:class}`~datafusion.object_store.AmazonS3` object and
register it on the context before issuing SQL that uses an `s3://` location. AWS
credentials are not SQL `OPTIONS`: `aws.*` is not a recognized SQL configuration
namespace.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This text is confusing. It reads as if it is answering a question in the issue rather than being user facing. There is no reason to think that aws.* would be a configuration namespace per se.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rewritten in 0d0a29e to explain creating and querying a table through the registered bucket store. Removed the namespace/error discussion from the guide and the #970 closure claim from the PR description.

Comment thread docs/source/user-guide/data-sources.md Outdated
- {py:class}`~datafusion.object_store.MicrosoftAzure`

```python
import os

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why import os?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It supported the two os.getenv calls in that example. Simplified this in 0d0a29e: AmazonS3 already reads environment configuration, so the import and both calls are removed; the required environment variables are stated above the example.

@emecii

emecii commented Sep 14, 2026

Copy link
Copy Markdown
Author

You're right: this PR was mis-scoped as closing #970. The issue explicitly requests aws.* SQL options and already reports successful Python registration. Removed the closure claim. Reproducing its SQL still gives Could not find config namespace "aws", even with a registered store. The CLI adds AWS option processing and object-store creation around SQL; Python's SQL binding delegates to the core context.

Pushed 0d0a29e with user-facing workflow text, simplified environment credential setup, and a network-free SQL registry test (26 object-store tests pass). Both docs examples also read three rows through loopback Moto with dummy credentials; this does not validate real AWS authentication or SDK credential-chain parity.

Is a docs-only change useful here, or should this PR instead implement CLI-style SQL AWS options to address #970? The main review concern remains open pending that direction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants