Har1sh-k commented on PR #66751:
URL: https://github.com/apache/airflow/pull/66751#issuecomment-5336038754
Thanks, both addressed, and I rewrote the PR description along the lines you
suggested.
**The table-name split.** You're right, and both examples reproduce exactly
as you posted them. The assembly now lives in `_quote_presto_table`, splitting
only on the dots outside a quoted identifier with the regex you proposed:
```python
_QUALIFIED_NAME_PART_RE = re.compile(r'"(?:[^"]|"")*"|[^.]+')
def _quote_presto_table(table: str) -> str:
return ".".join(_quote_presto_identifier(part) for part in
_QUALIFIED_NAME_PART_RE.findall(table))
```
`"my.table"` now stays `"my.table"` and `db."odd.name"` stays
`db."odd.name"`; plain `db.tbl` and bare `tbl` are byte-identical to before.
**The helper note.** Added `test_quote_presto_table` covering the four cases
you named (bare, pre-quoted, embedded doubled quote, and the dotted case), plus
per-component special characters and a trailing-SQL input that stays one inert
component. I also added the dotted cases to
`test_execute_quotes_table_identifier` so the behaviour is pinned at the
emitted-SQL level and not only at the helper. As a negative control I reverted
the helper body to the old `split(".")` and confirmed 6 of the new assertions
fail, so they discriminate rather than pass vacuously. Full module is 41
passed, 1 skipped (the pre-existing `AIRFLOW_RUNALL_TESTS` skip).
**One thing worth flagging, which I have deliberately left out of this
diff.** The quoted-dotted case cannot actually reach the `FROM` clause today,
because `HiveMetastoreHook.get_table` raw-splits the same string three lines
before the SQL is built:
```python
#
providers/apache/hive/src/airflow/providers/apache/hive/hooks/hive.py:703-704
if db == "default" and "." in table_name:
db, table_name = table_name.split(".")[:2]
```
So `table='"my.table"'` resolves to db `"my` / table `table"`, and
`db."odd.name"` resolves to db `db` / table `"odd`. Both fail at the metastore
lookup on line 164 of the operator before any SQL is generated. Running the
full matrix, the only inputs where this diff changes SQL that a real run would
actually emit are trailing-dot names: `db.tbl.` previously produced `FROM
db.tbl.""` and now produces `FROM db.tbl`.
I still think the helper should be correct on its own terms, since it is the
identifier-quoting primitive and the tests now pin it. But if you want the
quoted-dotted case to be genuinely reachable end to end, that means changing
how `HiveMetastoreHook` parses a qualified name, which affects every caller of
that hook and feels like a separate PR. Happy to open one if you would like it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
