Re: [PATCH] Add native PIVOT syntax for SQL Server/Oracle compatibility

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(at)sbcglobal(dot)net> wrote:
>
> Appreciate the feedback.
>
> I’ve incorporated all points below into a new patch.
>
> Thanks!
>
> Myles
>
> <0001-Add-native-PIVOT-syntax-support-for-SQL-Server-Oracl-1.patch>
>
>
>> On Nov 26, 2025, at 6:50 AM, Kirill Reshke <reshkekirill(at)gmail(dot)com> wrote:
>>
>> On Tue, 25 Nov 2025 at 13:11, Myles Lewis <myles93(at)sbcglobal(dot)net <mailto:myles93(at)sbcglobal(dot)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 detail
>>
>>> Error 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
>