fix: strip quotes and uppercase EXTRACT field name in deparser#289
Open
pyramation wants to merge 4 commits intomainfrom
Open
fix: strip quotes and uppercase EXTRACT field name in deparser#289pyramation wants to merge 4 commits intomainfrom
pyramation wants to merge 4 commits intomainfrom
Conversation
The EXTRACT SQL syntax handler was passing the A_Const sval value
directly, which gets single-quoted by QuoteUtils.formatEString().
This produced EXTRACT('epoch' FROM x) instead of the correct
EXTRACT(EPOCH FROM x).
Strip surrounding quotes and uppercase the field name to match
PostgreSQL's expected EXTRACT syntax.
Ref: constructive-io/constructive-db#748
Contributor
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Replace standalone extract.test.ts with fixture-based testing that follows the project's standard pattern: - Add __fixtures__/kitchen-sink/misc/extract.sql with EPOCH, YEAR, MONTH, DAY, HOUR, MINUTE, SECOND, CENTURY, MILLENNIUM, DOW, DOY, QUARTER, WEEK, TIMEZONE, TIMEZONE_HOUR, TIMEZONE_MINUTE - Regenerate generated.json and kitchen-sink test file - Remove ad-hoc misc/extract.test.ts
Add AGENTS.md with repo overview, package listing, setup instructions, and code conventions. Add .agents/skills/testing-fixtures/SKILL.md documenting the full fixture-based testing pipeline: how SQL fixtures get split into generated.json, how kitchen-sink tests are generated, and how to add new fixtures.
…with full script reference
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The SQL deparser's
FuncCallhandler forEXTRACTwas producingEXTRACT('epoch' FROM x)instead of the correctEXTRACT(EPOCH FROM x). The root cause: the first argument is anA_Constwith ansvalfield, andA_Constdeparsing wraps string values in single quotes viaQuoteUtils.formatEString(). The EXTRACT branch then passed this quoted string through verbatim.The fix strips surrounding single quotes and uppercases the field name before formatting the EXTRACT expression. This matches the behavior fixed in
constructive-io/constructive-db#748for the PL/pgSQL-native deparser.Adds a kitchen-sink fixture (
__fixtures__/kitchen-sink/misc/extract.sql) with 18 EXTRACT keyword variants (EPOCH, YEAR, MONTH, DAY, HOUR, MINUTE, SECOND, CENTURY, MILLENNIUM, DOW, DOY, QUARTER, WEEK, TIMEZONE, TIMEZONE_HOUR, TIMEZONE_MINUTE) using the project's standard fixture-based testing pipeline.Updates since last revision
AGENTS.mdat the repo root with project overview, package listing, setup/build/test instructions, and code conventions..agents/skills/testing-fixtures/SKILL.mddocumenting the full fixture-based testing pipeline (how SQL fixtures →generated.json→ kitchen-sink test files, how to add new fixtures, pretty-print test pattern, package scripts reference)..agents/skills/code-generation/SKILL.mddocumenting the protobuf-based code generation pipeline (build:protoacross 4 packages), type inference & narrowed type generation (pgsql-types), keyword list generation (@pgsql/quotes), and version-specific deparser generation.plpgsql-deparser) and transform package kitchen-sink/test:ast workflows, with per-package script reference tables.AGENTS.mdwith a Skills directory, comprehensive root and per-package script reference tables (organized by category: fixtures, code generation, version management, CLI), and a note about not hand-editing generated files.Review & Testing Checklist for Human
startsWith("'") && endsWith("'"). Verify this won't misfire on E-prefixed strings (E'...') — shouldn't happen since EXTRACT field names are simple keywords, but worth confirmingformatEStringnever adds theEprefix for plain alpha strings.date_partor any otherpg_catalog.*function handled in this block has a similar quoting problem with its string arguments (e.g.,TRIM,SUBSTRING,OVERLAYall use different arg patterns so likely fine, but worth a glance).cd packages/deparser && npx jest __tests__/kitchen-sink/misc-extract.test.tsto confirm all 18 EXTRACT variants pass the parse→deparse→reparse AST comparison.AGENTS.md,.agents/skills/testing-fixtures/SKILL.md, and.agents/skills/code-generation/SKILL.mdto confirm that documented commands, directory paths, and workflow descriptions match reality. All three were written from code reading — spot-check a few commands (e.g.,npm run build:protoinpackages/utils,npm run inferinpackages/pgsql-types,npm run keywordsinpackages/quotes) to verify they work as described.Notes
npm run kitchen-sink) rather than ad-hoc test files —generated.jsonand the kitchen-sink test file were regenerated from the new fixture SQLutils,traverse,transform,pg-ast,pgsql-types,proto-parser, andquotespackages — all based on reading the actual scripts, not existing documentation (there was none)constructive-io/constructive-db#748Link to Devin session: https://app.devin.ai/sessions/b02ffea25df94fd0a78385bbe5846415
Requested by: @pyramation