Skip to content

Leave unset storage values out of the Glue table parameters #890

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
laughingman7743 merged 2 commits into from
Sep 29, 2026
Merged

Leave unset storage values out of the Glue table parameters #890

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e86ec41
Leave unset storage values out of the Glue table parameters
laughingman7743 Sep 29, 2026
278e47c
Keep the storage descriptor's precedence over same-named table parame…
laughingman7743 Sep 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 24 additions & 15 deletions pyathena/glue.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,12 +243,14 @@ def list_databases(self, catalog_name: str | None) -> list[AthenaDatabase]:
def table_metadata(table: Mapping[str, Any]) -> AthenaTableMetadata:
"""Build the metadata Athena reports for a Glue table.

Athena flattens the storage descriptor into the table parameters: the
location and formats are always present, the SerDe library whenever
the descriptor has SerDe information, and SerDe parameters with a
``serde.param.`` prefix. The Glue description is not the table comment.
Glue keeps an Iceberg table's dropped and renamed columns, marked as
not current, which Athena leaves out.
Athena flattens the storage descriptor into the table parameters,
replacing parameters of the same name: the location and formats, the
SerDe library whenever the descriptor has SerDe information, and SerDe
parameters with a ``serde.param.`` prefix. A value the descriptor
leaves unset is left out rather than reported as None; an empty string

@laughingman7743 laughingman7743 Sep 29, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Self-review of the repair, round two (claims): CLEAN

Scope: the same range-diff, the repair commit message, the new docstring, the thread reply, and the PR body.

  • Docstring: "replacing parameters of the same name: the location and formats, the SerDe library whenever the descriptor has SerDe information" matches the code (storage always has the three keys, and the SerDe key only if "SerdeInfo" in descriptor). "A value the descriptor leaves unset is left out rather than reported as None; an empty string is kept" matches the is None branch and the view/s3_tables cases.
  • Commit message and reply: "as before" holds by the 2160-combination comparison with master.
  • PR body: still accurate. Its WHAT describes adding values "only when Glue sets them", and the table-parameter precedence is unchanged from before, so the description needs no correction.
  • Operations: unchanged; no request is added.

is kept. The Glue description is not the table comment. Glue keeps an
Iceberg table's dropped and renamed columns, marked as not current,
which Athena leaves out.

Args:
table: A ``Table`` from a Glue ``GetTable`` or ``GetTables`` response.
Expand All @@ -257,16 +259,23 @@ def table_metadata(table: Mapping[str, Any]) -> AthenaTableMetadata:
The table's metadata as Athena reports it.
"""
descriptor = table.get("StorageDescriptor") or {}
parameters = dict(table.get("Parameters") or {})
parameters["location"] = descriptor.get("Location")
parameters["inputformat"] = descriptor.get("InputFormat")
parameters["outputformat"] = descriptor.get("OutputFormat")
serde = descriptor.get("SerdeInfo") or {}
storage = {
"location": descriptor.get("Location"),
"inputformat": descriptor.get("InputFormat"),
"outputformat": descriptor.get("OutputFormat"),
}
if "SerdeInfo" in descriptor:
serde = descriptor["SerdeInfo"]
parameters["serde.serialization.lib"] = serde.get("SerializationLibrary")
parameters.update(
{f"serde.param.{k}": v for k, v in (serde.get("Parameters") or {}).items()}
)
storage["serde.serialization.lib"] = serde.get("SerializationLibrary")
parameters = dict(table.get("Parameters") or {})
for key, value in storage.items():
if value is None:
parameters.pop(key, None)

@laughingman7743 laughingman7743 Sep 29, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Self-review of the repair, round one (implementation behavior): CLEAN

Scope: git range-diff a86a180..e86ec41 a86a180..278e47c. The base is unchanged, and the repair adds one commit touching pyathena/glue.py table_metadata and tests/pyathena/test_glue.py. Both old objects exist locally.

Equivalence check: master's table_metadata (a86a180) was compared with this head over every combination of four table-level storage parameters (each present or absent) × Location/InputFormat/OutputFormat absent, '', or set × SerdeInfo absent, {}, with a library, with an empty library, or with only parameters. In all 2160 combinations, the new parameters equal the old parameters with the None-valued entries removed. So the precedence over same-named table parameters, the SerDe condition, and serde.param.* are unchanged, and the only difference is None becoming absent.

Callers: the same get_table/list_tables paths as in round one. The AthenaTableMetadata accessors (location, input_format, output_format, serde_serialization_lib, and file_format/row_format derived from them) return the same values as on master for every input.

Tests: the table_parameter_collision case fails on e86ec41 and passes here. Offline 11 passed, and the Glue-related AWS tests 35 passed at 278e47c.

else:
parameters[key] = value
parameters.update(
{f"serde.param.{k}": v for k, v in (serde.get("Parameters") or {}).items()}
)

def column(c: Mapping[str, Any]) -> dict[str, Any]:
return {k: c[k] for k in ("Name", "Type", "Comment") if k in c}
Expand Down
44 changes: 35 additions & 9 deletions tests/pyathena/test_glue.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,8 @@ def column(name, current):
"serde.param.field.delim": "\t",
},
),
# A view has empty SerDe information, which Athena still reports.
# A view has no formats and empty SerDe information; Athena reports
# only its empty location.
(
{
"Parameters": {"comment": "Presto View", "presto_view": "true"},
Expand All @@ -220,12 +221,9 @@ def column(name, current):
"comment": "Presto View",
"presto_view": "true",
"location": "",
"inputformat": None,
"outputformat": None,
"serde.serialization.lib": None,
},
),
# An Iceberg table has none, and Athena reports no SerDe library.
# An Iceberg table has no formats and no SerDe information.
(
{
"Parameters": {"table_type": "ICEBERG", "metadata_location": "s3://m"},
Expand All @@ -235,16 +233,44 @@ def column(name, current):
"table_type": "ICEBERG",
"metadata_location": "s3://m",
"location": "s3://bucket/iceberg",
"inputformat": None,
"outputformat": None,
},
),
# An S3 Tables table has empty formats, which Athena reports.
(
{
"Parameters": {"table_type": "ICEBERG"},
"StorageDescriptor": {
"Location": "s3://bucket--table-s3",
"InputFormat": "",
"OutputFormat": "",
},
},
{
"table_type": "ICEBERG",
"location": "s3://bucket--table-s3",

@laughingman7743 laughingman7743 Sep 29, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Self-review round two (claims, callers, operations): FINDINGS (PR description only, corrected)

Scope: base a86a180ebbe5e18920cd75802ea5350ce8d33f10 .. head e86ec41c753ca9359e1de8e71d7cf29be64ac3b4, the PR body, commit message, changed docstring, and the Glue fallback docs.

Claims checked:

  • The measurement table in the PR body matches the probe output: 5 GetTableMetadata calls per table, each returning a single shape; Hive tables equal; the view with only location: ''; Iceberg in AwsDataCatalog with only location; S3 Tables with inputformat: ''/outputformat: '' and no SerDe library.
  • "The AWS suite failed on every branch": only Require pandas 3.0 and pyarrow 22.0 #885's CI and local runs at the heads of Drop Python 3.10 support #884 and Require pandas 3.0 and pyarrow 22.0 #885 were observed. The body now states that.
  • "Answer throttled metadata requests from Glue in the cursor #803, unreleased": git tag --contains on the commit that added pyathena/glue.py is empty.
  • "The accessors use .get()" (pyathena/model.py location/input_format/output_format/serde_serialization_lib) and "awsathena_tblproperties carries the None entries" (pyathena/sqlalchemy/base.py reflects metadata.table_properties, which keeps every non-serde.param. key): both confirmed in the code.
  • The docstring now says values are added "whenever they are set (an empty string included)". This matches the is not None filter and the s3_tables and view test cases.

Existing callers: parameters/table_properties lose only None values on the Glue path. That makes them equal to what the Athena path returns now, and the accessors are unchanged.

Documentation: docs/sqlalchemy.md and the usage docs describe when Glue answers, not the parameter shape, so they need no change.

Operations: no request is added or removed; only the conversion of responses already fetched changes.

Evidence limits: 34 Glue-related tests passed locally against AWS at this head, and the full suite will run once the PR is Ready. The None-to-absent change was measured only in us-west-2.

"inputformat": "",
"outputformat": "",
},
),
# The descriptor replaces table parameters of the same name, also
# when it leaves the value unset.
(
{
"Parameters": {
"location": "s3://table-parameter",
"inputformat": "TableParameterInputFormat",
"serde.serialization.lib": "TableParameterSerDe",
},
"StorageDescriptor": {"Location": "", "SerdeInfo": {}},
},
{"location": ""},
),
],
ids=["hive", "view", "iceberg"],
ids=["hive", "view", "iceberg", "s3_tables", "table_parameter_collision"],
)
def test_table_metadata(self, table, expected_parameters):
# Glue responses measured against GetTableMetadata for the same tables in
# #786; Athena flattens them this way.
# #786 and #887; Athena flattens them this way.
table = {
"Name": "t",
"TableType": "EXTERNAL_TABLE",
Expand Down
Loading