The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: not tested
Documentation: tested, passed
I've tested the patched on 17devel/master and it is my feeling - especially
given the proliferation of the ORMs - that we need such thing in pgss. Thread
already took almost 3 years, so it would be pity to waste so much development
time of yours. Cfbot is green, and patch works very well for me. IMVHO
commitfest status should be even set to ready-for-comitter.
Given the:
SET query_id_const_merge = on;
SELECT pg_stat_statements_reset();
SELECT * FROM test WHERE a IN (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 11);
SELECT * FROM test WHERE a IN (1, 2, 3);
SELECT * FROM test WHERE a = ALL('{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11,
12}');
SELECT * FROM test WHERE a = ANY (ARRAY[11,10,9,8,7,6,5,4,3,2,1]);
The patch results in:
q | calls
-----------------------------------------------------+-------
SELECT * FROM test WHERE a = ALL($1) | 1
SELECT pg_stat_statements_reset() | 1
SELECT * FROM test WHERE a IN ($1, $2, $3) | 1
SELECT * FROM test WHERE a IN (... [10-99 entries]) | 2
Of course it's pity it doesn't collapse the below ones:
SELECT * FROM (VALUES (1), (2), (3), (4), (5), (6), (7), (8), (9), (10), (11))
AS t (num);
INSERT INTO dummy VALUES(1, 'text 1'),(2, 'text 2'),(3, 'text 3'),(4, 'text
3'),(5, 'text 3'),(6, 'text 3'),(7, 'text 3'),(8, 'text 3'),(9, 'text 3'),(10,
'text 3') ON CONFLICT (id) DO NOTHING;
PREPARE s3(int[], int[], int[], int[], int[], int[], int[], int[], int[],
int[], int[]) AS SELECT * FROM test WHERE
a = ANY ($1::int[]) OR
a = ANY ($2::int[]) OR
[..]
a = ANY ($11::int[]) ;
but given the convoluted thread history, it's understandable and as you stated
- maybe in future.
There's one additional benefit to this patch: the pg_hint_plan extension seems
to borrow pgss's generate_normalized_query(). So if that's changed in next
major release, the pg_hint_plan hint table (transparent plan rewrite using
table) will automatically benefit from generalization of the query string here
(imagine fixing plans for ORM that generate N {1,1024} number of IN() array
elements; today that would be N number of entries in the "hint_plan.hints"
table).
The new status of this patch is: Needs review
