Re: [PATCH] Add native PIVOT syntax for SQL Server/Oracle compatibility - Mailing list pgsql-hackers
| From | Myles Lewis |
|---|---|
| Subject | Re: [PATCH] Add native PIVOT syntax for SQL Server/Oracle compatibility |
| Date | |
| Msg-id | 8AA45018-F3D5-456F-B2E6-F613B5B635B3@sbcglobal.net Whole thread Raw |
| In response to | Re: [PATCH] Add native PIVOT syntax for SQL Server/Oracle compatibility (Myles Lewis <myles93@sbcglobal.net>) |
| List | pgsql-hackers |
I forgot to call out the key changes to the patch, based on your feedback (my apologies):
- Aggregate validation via catalog lookup — Replaced hardcoded aggregate name checks (SUM, COUNT, AVG, MIN, MAX) with a proper catalog lookup using func_get_detail(). This now supports user-defined aggregates, not just built-in ones.
- Immediate error reporting for SELECT * — Moved the ereport() directly inside the loop when A_Star is detected, rather than setting a flag and checking afterward.
- Added documentation comments — Added informal documentation in parse_clause.c explaining the PIVOT transformation, syntax, and behavior.
- Cleaned up test formatting — Removed numbered test sections (e.g., "Test 7.1:") to follow PostgreSQL test conventions.
On Nov 26, 2025, at 1:54 PM, Myles Lewis <myles93@sbcglobal.net> wrote:Appreciate the feedback.<0001-Add-native-PIVOT-syntax-support-for-SQL-Server-Oracl-1.patch>I’ve incorporated all points below into a new patch.Thanks!MylesOn Nov 26, 2025, at 6:50 AM, Kirill Reshke <reshkekirill@gmail.com> wrote:On Tue, 25 Nov 2025 at 13:11, Myles Lewis <myles93@sbcglobal.net> wrote:
I've developed a patch that adds native PIVOT syntax to PostgreSQL,
enabling SQL Server and Oracle-style pivot queries.
Example:
SELECT region
FROM sales
PIVOT (SUM(revenue) FOR quarter IN ('Q1', 'Q2', 'Q3', 'Q4'));
Key features:
- Parser-level transformation to FILTER aggregates
- No executor changes required
- Supports SUM, COUNT, AVG, MIN, MAX
- View creation with pg_get_viewdef() roundtrip
- Comprehensive regression tests (788 lines)
Patch attached.
Myles
Hi!+
+ if (IsA(lastField, A_Star))
+ {
+ hasStarExpand = true;
+ break;
+ }
+ }
+ }
+
+ if (hasStarExpand)
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("SELECT * is not allowed with PIVOT"),
+ errhint("Specify the columns you want in the SELECT list."),
+ parser_errposition(pstate, pstate->p_pivot_clause->location)));
+ }
You can ereport immediately inside the loop, since this is basically
`exit` syscall with some pg bookeepeing.+-- =============================================================================
+-- SECTION 7: CTE (Common Table Expression) TESTS
+-- =============================================================================
+
+-- Test 7.1: Simple CTE with PIVOT
You enamurated your test - this is something that is not done anywhere
else in PostgreSQL regression test suite... At least I do not find
anything similar.
To be clear, commenting on your tests is good, but enumeration is useless.+ /*
+ * Check for SELECT * - this is not allowed with PIVOT because we need
+ * explicit column selection for proper transformation.
+ */
I did not find an explanation, why exactly SELECT * is disallowed with
PIVOT. What exactly will not work if i do SELECT *, but would if i
manually specify all columns?
Commit msg:Features:
- Native PIVOT clause syntax: PIVOT (aggregate FOR column IN (values))
- Supports SUM, COUNT, AVG, MIN, MAX aggregates
- COUNT(*) special case supported
- String, integer, and date pivot values
- Subquery and JOIN sources
- CTE support (simple and nested)
- View creation with pg_get_viewdef() roundtrip
- Automatic GROUP BY generation from row identifiers
- Comprehensive error handling with source positions
I believe we do not need this info in commit msg at all. If a
committer commits something in PostgreSQL, it should work in all
cases. So, this contribution dont need list of things it works with -
It should work in every case.Transformation:
- PIVOT transforms to FILTER aggregates at parse time
- No executor changes required
- EXPLAIN shows expanded FILTER aggregates
I don't find that not making changes to executor is a benefit worth
mentioning in commit msg. This is implementation detailError cases handled:
- SELECT * with PIVOT (not allowed)
- Duplicate pivot values
- Invalid aggregate functions
- GROUP BY with PIVOT (not allowed)
- Column name conflicts
- Non-existent pivot/value columns
This just recalls changes to pivot.sql, adding no useful explanation.
It would be better to rewrite this part to indicate why exactly, say,
GROUP BY with PIVOT is not allowed (yet?) and if we have any plans on
improving this.Files modified:
- src/include/parser/kwlist.h: PIVOT keyword
- src/include/nodes/parsenodes.h: PivotClause, RangePivot nodes
- src/include/parser/parse_node.h: p_pivot_clause in ParseState
- src/backend/parser/gram.y: PIVOT grammar rules
- src/backend/parser/parse_clause.c: transformPivotClause()
- src/backend/parser/analyze.c: Phase 2 integration
- src/backend/utils/adt/ruleutils.c: View deparsing
- src/test/regress/sql/pivot.sql: Comprehensive test suite
- src/test/regress/expected/pivot.out: Expected output
- src/test/regress/parallel_schedule: Added pivot test
I believe we do not need this info in commit msg at all.
--
Best regards,
Kirill Reshke
Attachment
pgsql-hackers by date: