Home > mailing lists

Re: range_agg - Mailing list pgsql-hackers

From	Pavel Stehule
Subject	Re: range_agg
Date	November 19, 2019 12:16:47
Msg-id	CAFj8pRCYd8O32UWC5ZRWOhioOx4Z=AOnr9VO8fSTq6mdW--zOw@mail.gmail.com Whole thread Raw
In response to	Re: range_agg (Paul A Jungwirth <pj@illuminatedcomputing.com>)
Responses	Re: range_agg
List	pgsql-hackers

Tree view

čt 7. 11. 2019 v 3:36 odesílatel Paul A Jungwirth <pj@illuminatedcomputing.com> napsal:

On Wed, Nov 6, 2019 at 3:02 PM Paul A Jungwirth
<pj@illuminatedcomputing.com> wrote:
> On Thu, Sep 26, 2019 at 2:13 PM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> > Hello Paul, I've started to review this patch. Here's a few minor
> > things I ran across -- mostly compiler warnings (is my compiler too
> > ancient?).
> I just opened this thread to post a rebased set patches (especially
> because of the `const` additions to range functions). Maybe it's not
> that helpful since they don't include your changes yet but here they
> are anyway. I'll post some more with your changes shortly.

Here is another batch of patches incorporating your improvements. It
seems like almost all the warnings were about moving variable
declarations above any other statements. For some reason I don't get
warnings about that on my end (compiling on OS X):

platter:postgres paul$ gcc --version
Configured with:
--prefix=/Applications/Xcode.app/Contents/Developer/usr
--with-gxx-include-dir=/usr/include/c++/4.2.1
Apple clang version 11.0.0 (clang-1100.0.33.12)
Target: x86_64-apple-darwin18.6.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin

For configure I'm saying this:

./configure CFLAGS=-ggdb 5-Og -g3 -fno-omit-frame-pointer
--enable-tap-tests --enable-cassert --enable-debug
--prefix=/Users/paul/local

Any suggestions to get better warnings? On my other patch I got
feedback about the very same kind. I could just compile on Linux but
it's nice to work on this away from my desk on the laptop. Maybe
installing a real gcc is the way to go.

I tested last patches. I found some issues

1. you should not to try patch catversion.

2. there is warning

parse_coerce.c: In function ‘enforce_generic_type_consistency’:
parse_coerce.c:1975:11: warning: ‘range_typelem’ may be used uninitialized in this function [-Wmaybe-uninitialized]
1975 | else if (range_typelem != elem_typeid)

3. there are problems with pg_upgrade. Regress tests fails

command: "/home/pavel/src/postgresql.master/tmp_install/usr/local/pgsql/bin/pg_restore" --host /home/pavel/src/postgresql.master/src/b
pg_restore: connecting to database for restore
pg_restore: creating DATABASE "regression"
pg_restore: connecting to new database "regression"
pg_restore: connecting to database "regression" as user "pavel"
pg_restore: creating DATABASE PROPERTIES "regression"
pg_restore: connecting to new database "regression"
pg_restore: connecting to database "regression" as user "pavel"
pg_restore: creating pg_largeobject "pg_largeobject"
pg_restore: creating SCHEMA "fkpart3"
pg_restore: creating SCHEMA "fkpart4"
pg_restore: creating SCHEMA "fkpart5"
pg_restore: creating SCHEMA "fkpart6"
pg_restore: creating SCHEMA "mvtest_mvschema"
pg_restore: creating SCHEMA "regress_indexing"
pg_restore: creating SCHEMA "regress_rls_schema"
pg_restore: creating SCHEMA "regress_schema_2"
pg_restore: creating SCHEMA "testxmlschema"
pg_restore: creating TRANSFORM "TRANSFORM FOR integer LANGUAGE "sql""
pg_restore: creating TYPE "public.aggtype"
pg_restore: creating TYPE "public.arrayrange"
pg_restore: while PROCESSING TOC:
pg_restore: from TOC entry 1653; 1247 17044 TYPE arrayrange pavel
pg_restore: error: could not execute query: ERROR: pg_type array OID value not set when in binary upgrade mode
Command was:.
-- For binary upgrade, must preserve pg_type oid
SELECT pg_catalog.binary_upgrade_set_next_pg_type_oid('17044'::pg_catalog.oid);

-- For binary upgrade, must preserve pg_type array oid
SELECT pg_catalog.binary_upgrade_set_next_array_pg_type_oid('17045'::pg_catalog.oid);

CREATE TYPE "public"."arrayrange" AS RANGE (
subtype = integer[]
);

4. there is a problem with doc

echo "<!ENTITY version \"13devel\">"; \
echo "<!ENTITY majorversion \"13\">"; \
} > version.sgml
'/usr/bin/perl' ./mk_feature_tables.pl YES ../../../src/backend/catalog/sql_feature_packages.txt ../../../src/backend/catalog/sql_features.txt > features-supported.sgml
'/usr/bin/perl' ./mk_feature_tables.pl NO ../../../src/backend/catalog/sql_feature_packages.txt ../../../src/backend/catalog/sql_features.txt > features-unsupported.sgml
'/usr/bin/perl' ./generate-errcodes-table.pl ../../../src/backend/utils/errcodes.txt > errcodes-table.sgml
'/usr/bin/perl' ./generate-keywords-table.pl . > keywords-table.sgml
/usr/bin/xmllint --path . --noout --valid postgres.sgml
extend.sgml:281: parser error : Opening and ending tag mismatch: para line 270 and type
type of the ranges in an </type>anymultirange</type>.
^
extend.sgml:281: parser error : Opening and ending tag mismatch: sect2 line 270 and type
type of the ranges in an </type>anymultirange</type>.
^
extend.sgml:282: parser error : Opening and ending tag mismatch: sect1 line 270 and para
</para>
^
extend.sgml:324: parser error : Opening and ending tag mismatch: chapter line 270 and sect2
</sect2>
^

I am not sure how much is correct to use <literallayout class="monospaced"> in doc. It is used for ranges, and multiranges, but no in other places

All other looks well

Pavel

Thanks,
Paul

pgsql-hackers by date:

From: Amit Kapila
Date: 19 November 2019, 11:37:46
Subject: Re: logical decoding : exceeded maxAllocatedDescs for .spill files

From: Jinbao Chen
Date: 19 November 2019, 12:56:06
Subject: Re: Planner chose a much slower plan in hashjoin, using a large tableas the inner table.

Re: range_agg - Mailing list pgsql-hackers

Previous

Next