Re: range_agg

From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Paul A Jungwirth <pj(at)illuminatedcomputing(dot)com>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Jeff Davis <pgsql(at)j-davis(dot)com>, Pgsql Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: range_agg
Date: 2019-11-19 09:16:47
Message-ID: CAFj8pRCYd8O32UWC5ZRWOhioOx4Z=AOnr9VO8fSTq6mdW--zOw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi

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

> On Wed, Nov 6, 2019 at 3:02 PM Paul A Jungwirth
> <pj(at)illuminatedcomputing(dot)com> wrote:
> > On Thu, Sep 26, 2019 at 2:13 PM Alvaro Herrera <alvherre(at)2ndquadrant(dot)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
>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jinbao Chen 2019-11-19 09:56:06 Re: Planner chose a much slower plan in hashjoin, using a large table as the inner table.
Previous Message Amit Kapila 2019-11-19 08:37:46 Re: logical decoding : exceeded maxAllocatedDescs for .spill files