crprashant commented on PR #2415: URL: https://github.com/apache/age/pull/2415#issuecomment-4331123497
@jrgemignani thanks for the careful review — pushed [49a424e](https://github.com/apache/age/pull/2415/commits/49a424ead5a46eaae272b8496af4de2f1d8ed54e) addressing all four points: 1. **Redundant `pstrdup` removed.** `safe_keywords` already heap-copies via the `KEYWORD_STRDUP` macro (`pnstrdup((kw), strlen(kw))`), so the action is now `$$ = (char *) $1;` — the same pattern `schema_name` uses for its `reserved_keyword` branch. An inline comment explains why no further copy is needed. 2. **Read-back limitation documented in code.** Added a `NOTE:` block above `var_name_alias` explaining that `WITH 1 AS count RETURN count` is intentionally still rejected because broadening `expr_var`/`var_name` reintroduces ~156 shift/reduce conflicts, and linked it to the new follow-up issue **#2416**. 3. **Doc comment tightened.** Replaced "(and already accepted in `func_name` / `schema_name`)" with "(already accepted in `func_name`; `schema_name` accepts the broader `reserved_keyword`)" — accurately reflects that `schema_name` is a strict superset of what `var_name_alias` accepts. 4. **Regression coverage extended.** `regress/sql/reserved_keyword_alias.sql` now also exercises: - **Positive:** ``RETURN 1 AS `count` `` and ``WITH 1 AS `count` RETURN `count` `` — the backtick-quoted IDENTIFIER path, so future grammar refactors can't silently regress quoted identifiers when the unquoted form is also a keyword. - **Negative known-limitation:** `WITH 1 AS count RETURN count` is captured with its `syntax error at end of input` output and a comment pointing at #2416, giving the next contributor who tackles `expr_var` a precise file to update. Validation re-run on the amended commit: - Build/Regression (PG18, `COPT=-Werror`, `EXTRA_TESTS="pgvector fuzzystrmatch pg_trgm"`): **36/37** — only the pre-existing `age_upgrade` failure remains (same as `master`). - Python driver: all 4 suites green. - Go driver: all tests green. - Node.js driver: 31/31 green. - JDBC driver (`gradle clean test`): `BUILD SUCCESSFUL`, every `*Test PASSED`. - No new bison shift/reduce conflicts. Happy to keep iterating if anything else stands out. -- 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]
