From 9a24408eddfbd84c6a8ea03778716ce5c3653c28 Mon Sep 17 00:00:00 2001 From: Veesh Goldman Date: Tue, 2 Feb 2021 18:38:50 +0200 Subject: [PATCH 1/7] add support for covering index (INCLUDE) on PG producer - meaning, added an "include" option for the options option --- lib/SQL/Translator/Producer/PostgreSQL.pm | 16 +++++++++++----- t/47postgres-producer.t | 8 ++++++++ 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/lib/SQL/Translator/Producer/PostgreSQL.pm b/lib/SQL/Translator/Producer/PostgreSQL.pm index 7520563ff..4696a786f 100644 --- a/lib/SQL/Translator/Producer/PostgreSQL.pm +++ b/lib/SQL/Translator/Producer/PostgreSQL.pm @@ -296,6 +296,7 @@ sub create_table for my $index ( $table->get_indices ) { my ($idef, $constraints) = create_index($index, { generator => $generator, + postgres_version => $postgres_version, }); $idef and push @index_defs, $idef; push @constraint_defs, @$constraints; @@ -497,6 +498,7 @@ sub create_geometry_constraints { my $generator = _generator($options); my $table_name = $index->table->name; + my $postgres_version = $options->{postgres_version} || 0; my ($index_def, @constraint_defs); @@ -508,18 +510,22 @@ sub create_geometry_constraints { my @fields = $index->fields; return unless @fields; - my $index_using; - my $index_where; + my %index_extras; for my $opt ( $index->options ) { if ( ref $opt eq 'HASH' ) { foreach my $key (keys %$opt) { my $value = $opt->{$key}; next unless defined $value; if ( uc($key) eq 'USING' ) { - $index_using = "USING $value"; + $index_extras{using} = "USING $value"; } elsif ( uc($key) eq 'WHERE' ) { - $index_where = "WHERE $value"; + $index_extras{where} = "WHERE $value"; + } + elsif ( uc($key) eq 'INCLUDE' ) { + next unless $postgres_version >= 11; + my $value_list = join ', ', @$value; + $index_extras{include} = "INCLUDE ($value_list)" } } } @@ -536,7 +542,7 @@ sub create_geometry_constraints { elsif ( $type eq NORMAL ) { $index_def = 'CREATE INDEX ' . $generator->quote($name) . ' on ' . $generator->quote($table_name) . ' ' . - join ' ', grep { defined } $index_using, $field_names, $index_where; + join ' ', grep { defined } $index_extras{using}, $field_names, @index_extras{'include', 'where'}; } else { warn "Unknown index type ($type) on table $table_name.\n" diff --git a/t/47postgres-producer.t b/t/47postgres-producer.t index 9c50db736..1fae0a23c 100644 --- a/t/47postgres-producer.t +++ b/t/47postgres-producer.t @@ -655,6 +655,14 @@ is($view2_sql1, $view2_sql_replace, 'correct "CREATE OR REPLACE VIEW" SQL 2'); is($def, 'CREATE INDEX "myindex" on "foobar" ("bar", lower(foo))', 'index created w/ quotes'); } + { + my $index = $table->add_index(name => 'covering', fields => ['bar'], options => { include => [ 'lower(foo)' ] }); + my ($def) = SQL::Translator::Producer::PostgreSQL::create_index($index); + is($def, "CREATE INDEX covering on foobar (bar)", 'skip if postgres is too old'); + ($def) = SQL::Translator::Producer::PostgreSQL::create_index($index, { postgres_version => 11 }); + is($def, "CREATE INDEX covering on foobar (bar) INCLUDE (lower(foo))", 'index created'); + } + { my $constr = $table->add_constraint(name => 'constr', type => UNIQUE, fields => ['foo']); my ($def) = SQL::Translator::Producer::PostgreSQL::create_constraint($constr); From d4d748abd8659023213f754f0baefe341a1b6843 Mon Sep 17 00:00:00 2001 From: Veesh Goldman Date: Tue, 2 Feb 2021 18:43:11 +0200 Subject: [PATCH 2/7] more helpful input validation --- lib/SQL/Translator/Producer/PostgreSQL.pm | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/SQL/Translator/Producer/PostgreSQL.pm b/lib/SQL/Translator/Producer/PostgreSQL.pm index 4696a786f..abccdfc09 100644 --- a/lib/SQL/Translator/Producer/PostgreSQL.pm +++ b/lib/SQL/Translator/Producer/PostgreSQL.pm @@ -524,6 +524,7 @@ sub create_geometry_constraints { } elsif ( uc($key) eq 'INCLUDE' ) { next unless $postgres_version >= 11; + die 'Include list must be an arrayref' unless ref $value eq 'ARRAY'; my $value_list = join ', ', @$value; $index_extras{include} = "INCLUDE ($value_list)" } From 31e4531908b95c862a7c700a52008b9b35419305 Mon Sep 17 00:00:00 2001 From: Veesh Goldman Date: Sat, 7 Aug 2021 23:05:23 +0300 Subject: [PATCH 3/7] allow SQLT to parse INCLUDE in postgres index DDL --- lib/SQL/Translator/Parser/PostgreSQL.pm | 9 ++++++++- t/14postgres-parser.t | 16 +++++++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/lib/SQL/Translator/Parser/PostgreSQL.pm b/lib/SQL/Translator/Parser/PostgreSQL.pm index 37f657996..10754f1b6 100644 --- a/lib/SQL/Translator/Parser/PostgreSQL.pm +++ b/lib/SQL/Translator/Parser/PostgreSQL.pm @@ -236,7 +236,7 @@ create : CREATE temporary(?) TABLE table_id '(' create_definition(s? /,/) ')' ta 1; } -create : CREATE unique(?) /(index|key)/i index_name /on/i table_id using_method(?) '(' field_name(s /,/) ')' where_predicate(?) ';' +create : CREATE unique(?) /(index|key)/i index_name /on/i table_id using_method(?) '(' field_name(s /,/) ')' include_covering(?) where_predicate(?) ';' { my $table_info = $item{'table_id'}; my $schema_name = $table_info->{'schema_name'}; @@ -249,6 +249,7 @@ create : CREATE unique(?) /(index|key)/i index_name /on/i table_id using_method( fields => $item[9], method => $item{'using_method(?)'}[0], where => $item{'where_predicate(?)'}[0], + include => $item{'include_covering(?)'}[0] } ; } @@ -302,6 +303,9 @@ using_method : /using/i WORD { $item[2] } where_predicate : /where/i /[^;]+/ +include_covering : /include/i '(' covering_field_name(s /,/) ')' + { $item{'covering_field_name(s)'} } + create_definition : field | table_constraint | @@ -502,6 +506,8 @@ schema_name : NAME field_name : NAME +covering_field_name : NAME + double_quote: /"/ index_name : NAME @@ -1088,6 +1094,7 @@ sub parse { my @options = (); push @options, { using => $idata->{'method'} } if $idata->{method}; push @options, { where => $idata->{'where'} } if $idata->{where}; + push @options, { include => $idata->{'include'} } if $idata->{include}; my $index = $table->add_index( name => $idata->{'name'}, type => uc $idata->{'type'}, diff --git a/t/14postgres-parser.t b/t/14postgres-parser.t index 3c9dcd449..30a0d6833 100644 --- a/t/14postgres-parser.t +++ b/t/14postgres-parser.t @@ -78,6 +78,7 @@ baz $foo$, CREATE INDEX test_index1 ON t_test1 (f_varchar); CREATE INDEX test_index2 ON t_test1 USING hash (f_char, f_bool); CREATE INDEX test_index3 ON t_test1 USING hash (f_bigint, f_tz) WHERE f_bigint = '1' AND f_tz IS NULL; + CREATE INDEX test_index4 ON t_test1 USING hash (f_bigint, f_tz) include (f_bool) WHERE f_bigint = '1' AND f_tz IS NULL; alter table t_test1 add f_fk2 integer; @@ -406,7 +407,7 @@ is( $trigger->action, 'EXECUTE PROCEDURE foo()', "Correct action for trigger"); # test index my @indices = $t1->get_indices; -is(scalar @indices, 3, 'got three indexes'); +is(scalar @indices, 4, 'got three indexes'); my $t1_i1 = $indices[0]; is( $t1_i1->name, 'test_index1', 'First index is "test_index1"' ); @@ -427,4 +428,17 @@ is_deeply( 'Index is using hash method and has predicate right' ); +my $t1_i4 = $indices[3]; +is( $t1_i4->name, 'test_index4', 'Fourth index is "test_index4"' ); +is( join(',', $t1_i4->fields), 'f_bigint,f_tz', 'Index is on fields "f_bigint, f_tz"' ); +is_deeply( + [ $t1_i4->options ], + [ + { using => 'hash' }, + { where => "f_bigint = '1' AND f_tz IS NULL" }, + { include => [ 'f_bool' ] } + ], + 'Index is using hash method and has predicate right and include INCLUDE' +); + done_testing; From c5831fb691394ccdff04905854b40c636a890f78 Mon Sep 17 00:00:00 2001 From: Veesh Goldman Date: Sat, 7 Aug 2021 23:09:34 +0300 Subject: [PATCH 4/7] add a change line --- Changes | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Changes b/Changes index d020bfec0..5b7ce6ca2 100644 --- a/Changes +++ b/Changes @@ -1,5 +1,7 @@ Changes for SQL::Translator + * Support INCLUDE on indices for Pg (producer + parser) + 1.62 - 2020-09-14 * Update Pg support to allow version 12 (still supporting back to 7.4) From babe2cc903aab70f49dfdd789f96957940c995b8 Mon Sep 17 00:00:00 2001 From: Veesh Goldman Date: Sat, 7 Aug 2021 23:11:27 +0300 Subject: [PATCH 5/7] more thorough test for producing a covering index --- t/47postgres-producer.t | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/47postgres-producer.t b/t/47postgres-producer.t index 1fae0a23c..ec6fbf234 100644 --- a/t/47postgres-producer.t +++ b/t/47postgres-producer.t @@ -656,11 +656,11 @@ is($view2_sql1, $view2_sql_replace, 'correct "CREATE OR REPLACE VIEW" SQL 2'); } { - my $index = $table->add_index(name => 'covering', fields => ['bar'], options => { include => [ 'lower(foo)' ] }); + my $index = $table->add_index(name => 'covering', fields => ['bar'], options => { include => [ 'lower(foo)', 'baz' ] }); my ($def) = SQL::Translator::Producer::PostgreSQL::create_index($index); is($def, "CREATE INDEX covering on foobar (bar)", 'skip if postgres is too old'); ($def) = SQL::Translator::Producer::PostgreSQL::create_index($index, { postgres_version => 11 }); - is($def, "CREATE INDEX covering on foobar (bar) INCLUDE (lower(foo))", 'index created'); + is($def, "CREATE INDEX covering on foobar (bar) INCLUDE (lower(foo), baz)", 'index created'); } { From f813e7f0a13d93b83ca6aa7845d4202b322d7e7b Mon Sep 17 00:00:00 2001 From: Veesh Goldman Date: Sun, 8 Aug 2021 00:41:27 +0300 Subject: [PATCH 6/7] update docs to include INCLUDE --- lib/SQL/Translator/Parser/PostgreSQL.pm | 5 +++-- lib/SQL/Translator/Producer/PostgreSQL.pm | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/SQL/Translator/Parser/PostgreSQL.pm b/lib/SQL/Translator/Parser/PostgreSQL.pm index 10754f1b6..6c537067d 100644 --- a/lib/SQL/Translator/Parser/PostgreSQL.pm +++ b/lib/SQL/Translator/Parser/PostgreSQL.pm @@ -51,11 +51,12 @@ Table: [ DEFERRABLE | NOT DEFERRABLE ] [ INITIALLY DEFERRED | INITIALLY IMMEDIATE ] -Index: -(http://www.postgresql.org/docs/view.php?version=7.3&idoc=1&file=sql-createindex.html) +Index : +(http://www.postgresql.org/docs/11/sql-createindex.html) CREATE [ UNIQUE ] INDEX index_name ON table [ USING acc_method ] ( column [ ops_name ] [, ...] ) + [ INCLUDE ( column [, ...] ) ] [ WHERE predicate ] CREATE [ UNIQUE ] INDEX index_name ON table [ USING acc_method ] ( func_name( column [, ... ]) [ ops_name ] ) diff --git a/lib/SQL/Translator/Producer/PostgreSQL.pm b/lib/SQL/Translator/Producer/PostgreSQL.pm index abccdfc09..22ba7317b 100644 --- a/lib/SQL/Translator/Producer/PostgreSQL.pm +++ b/lib/SQL/Translator/Producer/PostgreSQL.pm @@ -129,6 +129,7 @@ and table_constraint is: CREATE [ UNIQUE ] INDEX index_name ON table [ USING acc_method ] ( column [ ops_name ] [, ...] ) + [ INCLUDE ( column [, ...] ) ] [ WHERE predicate ] CREATE [ UNIQUE ] INDEX index_name ON table [ USING acc_method ] ( func_name( column [, ... ]) [ ops_name ] ) From cdc2a285b1ac3e434f588f4996e5b42d8cfea013 Mon Sep 17 00:00:00 2001 From: Veesh Goldman Date: Mon, 9 Aug 2021 23:32:16 +0300 Subject: [PATCH 7/7] update Parser::PostgreSQL's docs to be clearer - use /current for PG doc links, also --- lib/SQL/Translator/Parser/PostgreSQL.pm | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/SQL/Translator/Parser/PostgreSQL.pm b/lib/SQL/Translator/Parser/PostgreSQL.pm index 6c537067d..16d99548b 100644 --- a/lib/SQL/Translator/Parser/PostgreSQL.pm +++ b/lib/SQL/Translator/Parser/PostgreSQL.pm @@ -15,10 +15,10 @@ SQL::Translator::Parser::PostgreSQL - parser for PostgreSQL =head1 DESCRIPTION The grammar was started from the MySQL parsers. Here is the description -from PostgreSQL: +from PostgreSQL, truncated to what's currently supported (patches welcome, of course) : Table: -(http://www.postgresql.org/docs/view.php?version=7.3&idoc=1&file=sql-createtable.html) +(http://www.postgresql.org/docs/current/sql-createtable.html) CREATE [ [ LOCAL ] { TEMPORARY | TEMP } ] TABLE table_name ( { column_name data_type [ DEFAULT default_expr ] @@ -52,7 +52,7 @@ Table: [ INITIALLY DEFERRED | INITIALLY IMMEDIATE ] Index : -(http://www.postgresql.org/docs/11/sql-createindex.html) +(http://www.postgresql.org/docs/current/sql-createindex.html) CREATE [ UNIQUE ] INDEX index_name ON table [ USING acc_method ] ( column [ ops_name ] [, ...] ) @@ -81,7 +81,7 @@ Alter table: ALTER TABLE table OWNER TO new_owner -View table: +View : CREATE [ OR REPLACE ] VIEW view [ ( column name list ) ] AS SELECT query