Re: Postgres picks suboptimal index after building of an extended statistics

[Andrey Lepikhov]

On 12/8/21 04:26, Tomas Vondra wrote:

On 8/11/21 2:48 AM, Peter Geoghegan wrote:

On Wed, Jun 23, 2021 at 7:19 AM Andrey V. Lepikhov
I agree the current behavior is unfortunate, but I'm not convinced the 
proposed patch is fixing the right place - doesn't this mean the index 
costing won't match the row estimates displayed by EXPLAIN?

I rewrote the patch. It's now simpler and shorter. May be more convenient.
Also, it includes a regression test to detect the problem in future.


I wonder if we should teach clauselist_selectivity about UNIQUE indexes, 
and improve the cardinality estimates directly, not just costing for 
index scans.
I tried to implement this in different ways. But it causes additional 
overhead and code complexity - analyzing a list of indexes and match 
clauses of each index with input clauses in each selectivity estimation.

I don't like that way and propose a new patch in attachment.

--
regards,
Andrey Lepikhov
Postgres Professional
From 41ce6007cd552afd1a73983f0b9c9cac0e125d58 Mon Sep 17 00:00:00 2001
From: "Andrey V. Lepikhov" 
Date: Mon, 30 Aug 2021 11:21:57 +0500
Subject: [PATCH] Estimating number of fetched rows in a btree index we save
 selectivity estimation in the costs structure. It will be used by the
 genericcostestimate routine as a top bound for estimation of total tuples,
 visited in the main table.

This code fix the problem with unique index, when we know for sure
that no more than one tuple can be fetched, but clauselist_selectivity
gives us much less accurate estimation because of many possible reasons.
A regression test is added as a demonstration of the problem.
---
 src/backend/utils/adt/selfuncs.c| 18 ++--
 src/test/regress/expected/stats_ext.out | 38 +
 src/test/regress/sql/stats_ext.sql  | 34 ++
 3 files changed, 88 insertions(+), 2 deletions(-)

diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index c2aeb4b947..dd1cadad61 100644
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -6074,6 +6074,14 @@ genericcostestimate(PlannerInfo *root,
 */
numIndexTuples = rint(numIndexTuples / num_sa_scans);
}
+   else if (costs->indexSelectivity > 0. &&
+   indexSelectivity > costs->indexSelectivity)
+   /*
+* If caller give us an estimation of amount of fetched index 
tuples,
+* it could give the selectivity estimation. In this case 
amount of
+* returned tuples can't be more than amount of fetched tuples.
+*/
+   indexSelectivity = costs->indexSelectivity;
 
/*
 * We can bound the number of tuples by the index size in any case. 
Also,
@@ -6258,6 +6266,7 @@ btcostestimate(PlannerInfo *root, IndexPath *path, double 
loop_count,
boolfound_is_null_op;
double  num_sa_scans;
ListCell   *lc;
+   Selectivity btreeSelectivity;
 
/*
 * For a btree scan, only leading '=' quals plus inequality quals for 
the
@@ -6362,19 +6371,23 @@ btcostestimate(PlannerInfo *root, IndexPath *path, 
double loop_count,
/*
 * If index is unique and we found an '=' clause for each column, we can
 * just assume numIndexTuples = 1 and skip the expensive
-* clauselist_selectivity calculations.  However, a ScalarArrayOp or
+* clauselist_selectivity calculations. However, a ScalarArrayOp or
 * NullTest invalidates that theory, even though it sets eqQualHere.
+* Value of btreeSelectivity is used as a top bound for selectivity
+* estimation of returned tuples in the genericcostestimate routine.
 */
if (index->unique &&
indexcol == index->nkeycolumns - 1 &&
eqQualHere &&
!found_saop &&
!found_is_null_op)
+   {
numIndexTuples = 1.0;
+   btreeSelectivity = 1. / index->rel->tuples;
+   }
else
{
List   *selectivityQuals;
-   Selectivity btreeSelectivity;
 
/*
 * If the index is partial, AND the index predicate with the
@@ -6402,6 +6415,7 @@ btcostestimate(PlannerInfo *root, IndexPath *path, double 
loop_count,
 */
MemSet(&costs, 0, sizeof(costs));
costs.numIndexTuples = numIndexTuples;
+   costs.indexSelectivity = btreeSelectivity;
 
genericcostestimate(root, path, loop_count, &costs);
 
diff --git a/src/test/regress/expected/stats_ext.out 
b/src/test/regress/expected/stats_ext.out
index 7524e65142..b90463821f 100644
--- a/src/test/regress/expected/stats_ext.out
+++ b/src/test/regress/expected/stats_ext.out
@@ -1602,3 +1602,41 @@ NOTICE:  drop cascades to 2 other objects
 DETAIL:  drop cascades to table tststats.priv_test_tbl
 drop cascades to view tststats.priv_test_view
 DROP US

Re: Separate out FileSet from SharedFileSet (was Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o)

[Amit Kapila]

On Mon, Aug 30, 2021 at 6:42 AM Masahiko Sawada  wrote:
>
> On Fri, Aug 27, 2021 at 2:25 PM Dilip Kumar  wrote:
> >
> > On Thu, Aug 26, 2021 at 2:10 PM Amit Kapila  wrote:
> >>
> >>
> >> Thanks, Sawada-San and Dilip for confirmation. I would like to commit
> >> this and the second patch (the second one still needs some more
> >> testing and review) for PG-15 as there is no bug per-se related to
> >> this work in PG-14 but I see an argument to commit this for PG-14 to
> >> keep the code (APIs) consistent. What do you think? Does anybody else
> >> have any opinion on this?
> >>
> >
> > IMHO, this is a fair amount of refactoring and this is actually an 
> > improvement patch so we should push it to v15.
>
> I think so too.
>

I have pushed the first patch in this series.

-- 
With Regards,
Amit Kapila.

Re: Added schema level support for publication.

[Amit Kapila]

On Sun, Aug 29, 2021 at 7:22 AM vignesh C  wrote:
>
> On Sat, Aug 28, 2021 at 3:19 PM Amit Kapila  wrote:
> >
> > On Fri, Aug 27, 2021 at 6:09 PM vignesh C  wrote:
> > >
> >
> > What do you mean by update should fail? I think all the relations in
> > RelationSyncCache via rel_sync_cache_publication_cb because you have
> > updated pg_publication_schema and that should register syscache
> > invalidation.
>
> By update should fail, I meant the updates without setting replica
> identity before creating the decoding context. The scenario is like
> below (all in the same session, the subscription is not created):
> create schema sch1;
> create table sch1.t1(c1 int);
> insert into sch1.t1 values(10);
> # Before updating we will check CheckCmdReplicaIdentity, as there are
> no publications on this table rd_pubactions will be set accordingly in
> relcache entry.
> update sch1.t1 set c1 = 11;
> # Now we will create the publication after rd_pubactions has been set
> in the cache. Now when we create this publication we should invalidate
> the relations present in the schema, this is required so that when the
> next update happens, we should check the publication actions again in
> CheckCmdReplicaIdentity and fail the update which does not  set
> replica identity after the publication is created.
> create publication pub1 for all tables in schema sch1;
> # After the above publication is created the relations present in this
> schema will be invalidated. Now we will check the publication actions
> again in CheckCmdReplicaIdentity and the update will fail.
> update sch1.t1 set c1 = 11;
> ERROR:  cannot update table "t1" because it does not have a replica
> identity and publishes updates
> HINT:  To enable updating the table, set REPLICA IDENTITY using ALTER TABLE.
>

Okay, I got it but let's add few comments in the code related to it.
Also, I noticed that the code in InvalidatePublicationRels() already
exists in AlterPublicationOptions(). You can try to refactor the
existing code as a separate initial patch.

BTW, I noticed that "for all tables", we don't register invalidations
in the above scenario, and then later that causes conflict on the
subscriber. I think that is a bug in the current code and we can deal
with that separately.

-- 
With Regards,
Amit Kapila.

Re: replace IDENTIFY_SYSTEM code in receivelog.c with RunIdentifySystem()

[Michael Paquier]

On Mon, Aug 30, 2021 at 11:00:40AM +0530, Bharath Rupireddy wrote:
> 1) ReceiveXlogStream in receivelog.c has a duplicate code to execute
> IDENTIFY_SYSTEM replication command on the server which can be
> replaced with RunIdentifySystem().

I have looked at that.

> 2) bool returning ReceiveXlogStream() in pg_receivewal.c is being used
> without type-casting its return return value which might generate a
> warning with some compilers. This kind of type-casting is more common
> in other places in the postgres code base.

This is usually a pattern used for Coverity, to hint it that we don't
care about the error code in a given code path.  IMV, that's not
something to bother about for older code.

> Attaching a patch to fix the above. Thoughts?

The original refactoring of IDENTIFY_SYSTEM is from 0c013e08, and it
feels like I just missed ReceiveXlogStream().  What you have here is
an improvement.

+   if (!RunIdentifySystem(conn, &sysidentifier, &servertli, NULL, NULL))
{
-   pg_log_error("could not send replication command \"%s\": %s",
-"IDENTIFY_SYSTEM", PQerrorMessage(conn));
-   PQclear(res);
+   pg_free(sysidentifier);
return false;

Here you want to free sysidentifier only if it has been set, and
RunIdentifySystem() may fail before doing that, so you should assign
NULL to sysidentifier when it is declared.
--
Michael


signature.asc
Description: PGP signature

Re: Separate out FileSet from SharedFileSet (was Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o)

[Masahiko Sawada]

On Fri, Aug 27, 2021 at 3:34 PM Dilip Kumar  wrote:
>
> On Fri, Aug 27, 2021 at 10:56 AM houzj.f...@fujitsu.com
>  wrote:
> >
> > On Thu, Aug 26, 2021 2:18 PM Dilip Kumar  wrote:
> >
> > > The patch looks good to me, I have rebased 0002 atop
> > > this patch and also done some cosmetic fixes in 0002.
> >
> > Here are some comments for the 0002 patch.
> >
> > 1)
> >
> > - * toplevel transaction. Each subscription has a separate set of files.
> > + * toplevel transaction. Each subscription has a separate files.
> >
> > a separate files => a separate file
>
> Done
>
> > 2)
> > +* Otherwise, just open it file for writing, in append mode.
> >  */
> >
> > open it file => open the file
>
> Done
>
> >
> > 3)
> > if (subxact_data.nsubxacts == 0)
> > {
> > -   if (ent->subxact_fileset)
> > -   {
> > -   cleanup_subxact_info();
> > -   FileSetDeleteAll(ent->subxact_fileset);
> > -   pfree(ent->subxact_fileset);
> > -   ent->subxact_fileset = NULL;
> > -   }
> > +   cleanup_subxact_info();
> > +   BufFileDeleteFileSet(stream_fileset, path, true);
> > +
> >
> > Before applying the patch, the code only invoke cleanup_subxact_info() when 
> > the
> > file exists. After applying the patch, it will invoke cleanup_subxact_info()
> > either the file exists or not. Is it correct ?
>
> I think this is just structure resetting at the end of the stream.
> Earlier the hash was telling us whether we have ever dirtied that
> structure or not but now we are not maintaining that hash so we just
> reset it at the end of the stream.  I don't think its any bad, in fact
> I think this is much cheaper compared to maining the hash.
>
> >
> > 4)
> > /*
> > -* If this is the first streamed segment, the file must not exist, 
> > so make
> > -* sure we're the ones creating it. Otherwise just open the file for
> > -* writing, in append mode.
> > +* If this is the first streamed segment, create the changes file.
> > +* Otherwise, just open it file for writing, in append mode.
> >  */
> > if (first_segment)
> > -   {
> > ...
> > -   if (found)
> > -   ereport(ERROR,
> > -   
> > (errcode(ERRCODE_PROTOCOL_VIOLATION),
> > -errmsg_internal("incorrect 
> > first-segment flag for streamed replication transaction")));
> > ...
> > -   }
> > +   stream_fd = BufFileCreateFileSet(stream_fileset, path);
> >
> >
> > Since the function BufFileCreateFileSet() doesn't check the file's 
> > existence,
> > the change here seems remove the file existence check which the old code 
> > did.
>
> Not really, we were just doing a sanity check of the in memory hash
> entry, now we don't maintain that so we don't need to do that.

Thank you for updating the patch!

The patch looks good to me except for the below comment:

+/* Delete the subxact file, if it exist. */
+subxact_filename(path, subid, xid);
+BufFileDeleteFileSet(stream_fileset, path, true);

s/it exist/it exists/

Regards,

-- 
Masahiko Sawada
EDB:  https://www.enterprisedb.com/

Re: Fix around conn_duration in pgbench

[Tatsuo Ishii]

> On Mon, 30 Aug 2021 14:22:49 +0900 (JST)
> Tatsuo Ishii  wrote:
> 
>> >> Anyway, I changed the status of this patch to "Waiting on Author" in CF.
>> > 
>> > I returned the status to "Ready for Committer". 
>> > Could you please review this?
>> 
>> According to the patch tester, the patch does not apply.
> 
> Well, that's because both the patch for PG14 and one for PG13
> are discussed here.

Oh, ok. So the patch tester is not smart enough to identify each patch
for particular branches.
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp

Re: create table like: ACCESS METHOD

[Michael Paquier]

On Fri, Aug 27, 2021 at 02:38:43PM +0900, Michael Paquier wrote:
> +CREATE ACCESS METHOD heapdup TYPE TABLE HANDLER heap_tableam_handler;
> +CREATE TABLE likeam() USING heapdup;
> +CREATE TABLE likeamlike(LIKE likeam INCLUDING ALL);
> Rather than creating a custom AM in this test path, I would be
> tempted to move that to create_am.sql.

+   /* ACCESS METHOD doesn't apply and isn't copied for partitioned tables 
*/
+   if ((table_like_clause->options & CREATE_TABLE_LIKE_ACCESS_METHOD) != 0 
&&
+   !cxt->ispartitioned)
+   cxt->accessMethod = get_am_name(relation->rd_rel->relam);

If the new table is partitioned, this would work.  Now I think that we
should also add here a (relation->rd_rel->relkind == RELKIND_RELATION)
to make sure that we only copy an access method if the original
relation is a table.  Note that the original relation could be as well
a view, a foreign table or a composite type.

@@ -349,6 +351,9 @@ transformCreateStmt(CreateStmt *stmt, const char 
*queryString)
[...]
+   if (cxt.accessMethod != NULL)
+   stmt->accessMethod = cxt.accessMethod;

This bit is something I have been chewing on a bit.  It means that if
we find out an AM to copy from any of the LIKE clauses, we would
blindly overwrite the AM defined in an existing CreateStmt.  We could
also argue in favor of keeping the original AM defined by USING from
the query rather than having an error.  This means to check that
stmt->accessMethod is overwritten only if NULL at this point.  Anyway,
the patch is wrong with this implementation.

This makes me actually wonder if this patch is really a good idea at
the end.  The interactions with USING and LIKE would be confusing to
the end-user one way or the other.  The argument of upthread regarding
INCLUDING ALL or INCLUDING ACCESS METHOD with multiple original
relations also goes in this sense.  If we want to move forward here, I
think that we should really be careful and have a clear definition
behind all those corner cases.  The patch fails this point for now.
--
Michael


signature.asc
Description: PGP signature

Re: Fix around conn_duration in pgbench

[Yugo NAGATA]

On Mon, 30 Aug 2021 14:22:49 +0900 (JST)
Tatsuo Ishii  wrote:

> >> Anyway, I changed the status of this patch to "Waiting on Author" in CF.
> > 
> > I returned the status to "Ready for Committer". 
> > Could you please review this?
> 
> According to the patch tester, the patch does not apply.

Well, that's because both the patch for PG14 and one for PG13
are discussed here.

> --
> Tatsuo Ishii
> SRA OSS, Inc. Japan
> English: http://www.sraoss.co.jp/index_en.php
> Japanese:http://www.sraoss.co.jp
> 
> 


-- 
Yugo NAGATA 

replace IDENTIFY_SYSTEM code in receivelog.c with RunIdentifySystem()

[Bharath Rupireddy]

Hi,

I see a couple of improvements to receivelog.c and pg_receivewal.c:

1) ReceiveXlogStream in receivelog.c has a duplicate code to execute
IDENTIFY_SYSTEM replication command on the server which can be
replaced with RunIdentifySystem().
2) bool returning ReceiveXlogStream() in pg_receivewal.c is being used
without type-casting its return return value which might generate a
warning with some compilers. This kind of type-casting is more common
in other places in the postgres code base.

Attaching a patch to fix the above. Thoughts?

Regards,
Bharath Rupireddy.


v1-0001-replace-IDENTIFY_SYSTEM-code-in-receivelog.c-with.patch
Description: Binary data

Re: Fix around conn_duration in pgbench

[Tatsuo Ishii]

>> Anyway, I changed the status of this patch to "Waiting on Author" in CF.
> 
> I returned the status to "Ready for Committer". 
> Could you please review this?

According to the patch tester, the patch does not apply.
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp

Re: Using COPY FREEZE in pgbench

[Tatsuo Ishii]

> I tested this with -s100 and got similar results, but with -s1000 it
> was more like 1.5x faster:
> 
> master:
> done in 111.33 s (drop tables 0.00 s, create tables 0.01 s,
> client-side generate 52.45 s, vacuum 32.30 s, primary keys 26.58 s)
> 
> patch:
> done in 74.04 s (drop tables 0.46 s, create tables 0.04 s, client-side
> generate 51.81 s, vacuum 2.11 s, primary keys 19.63 s)
> 
> Nice!
> 
> Regards,
> Dean

If there's no objection, I am going to commit/push to master branch in
early September.
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp

Re: create table like: ACCESS METHOD

[Michael Paquier]

On Fri, Aug 27, 2021 at 12:37:59PM +0200, Vik Fearing wrote:
> It seems like this should error to me:
> 
> CREATE ACCESS METHOD heapdup TYPE TABLE HANDLER heap_tableam_handler;
> CREATE TABLE likeam1() USING heap;
> CREATE TABLE likeam2() USING heapdup;
> CREATE TABLE likeamlike(
> LIKE likeam1 INCLUDING ACCESS METHOD,
> LIKE likeam2 INCLUDING ACCESS METHOD
> );
> 
> At the very least, the documentation should say that the last one wins.

An error may be annoying once you do an INCLUDING ALL with more than
one relation, no?  I'd be fine with just documenting that the last one
wins.
--
Michael


signature.asc
Description: PGP signature

Re: simplifying foreign key/RI checks

[Corey Huinker]

>
> Rebased patches attached.


I'm reviewing the changes since v6, which was my last review.

Making ExecLockTableTuple() it's own function makes sense.
Snapshots are now accounted for.
The changes that account for n-level partitioning makes sense as well.

Passes make check-world.
Not user facing, so no user documentation required.
Marking as ready for committer again.

Re: jff: checksum algorithm is not as intended

[Tom Lane]

Yura Sokolov  writes:
> Single round function is written as:

> #define CHECKSUM_COMP(checksum, value) do {\
>  uint32 __tmp = (checksum) ^ (value);\
>  (checksum) = __tmp * FNV_PRIME ^ (__tmp >> 17);\
> } while (0)

> And looks like it was intended to be
>  (checksum) = (__tmp * FNV_PRIME) ^ (__tmp >> 17);

I'm not following your point?  Multiplication binds tighter than XOR
in C, see e.g.

https://en.wikipedia.org/wiki/Operators_in_C_and_C%2B%2B#Operator_precedence

So those sure look equivalent from here.

regards, tom lane

RE: Added schema level support for publication.

[houzj.f...@fujitsu.com]

On Friday, August 27, 2021 2:13 PM vignesh C  wrote:
> 
> I have implemented this in the 0003 patch, I have kept it separate to reduce 
> the
> testing effort and also it will be easier if someone disagrees with the 
> syntax. I
> will merge it to the main patch later based on the feedback. Attached v22 
> patch
> has the changes for the same.
> Thoughts?

Hi,

Here are some comments for the new version patches.

About  0001
1)
+   rel->relpersistence = RELPERSISTENCE_PERMANENT;

It seems we don't need to set this since makeRangeVarFromNameList()
already set it.


2)
+   if (!relids || !schemarelids)
+   tables = list_concat(relids, schemarelids);
+   else
+   tables = list_concat_unique_oid(relids, 
schemarelids);
+   }

It seems we can simplify the above code like the following: 
tables = list_concat_unique_oid(relids, schemarelids);


3)
+   relids = GetPublicationRelations(pubform->oid,
+   
 PUBLICATION_PART_ALL);
+   schemarelids = GetAllSchemasPublicationRelations(pubform->oid,
+   
 PUBLICATION_PART_ALL);
+   relids = list_concat(relids, schemarelids);

should we invoke list_concat_unique_oid here ?

4)

+   search_path = fetch_search_path(false);
+   if (search_path == NIL) /* nothing valid in 
search_path? */

It might be better to list_free(search_path) when not used.

5)
+   if (list_length(pubobj->name) == 1 &&
+   (strcmp(relname, "CURRENT_SCHEMA") == 0))
+   ereport(ERROR,
+   errcode(ERRCODE_SYNTAX_ERROR),
+   errmsg("invalid relation name 
at or near"),
+   parser_errposition(pstate, 
pubobj->location));

Maybe we don't need this check, because it will report an error in
OpenTableList() anyway, "relation "CURRENT_SCHEMA" does not exist" , and that
message seems readable to me.


About  0002
6)

diff --git a/src/test/subscription/t/001_rep_changes.pl 
b/src/test/subscription/t/001_rep_changes.pl
index 0c84d87873..0a479dfe36 100644
--- a/src/test/subscription/t/001_rep_changes.pl
+++ b/src/test/subscription/t/001_rep_changes.pl
@@ -6,7 +6,7 @@ use strict;
 use warnings;
 use PostgresNode;
 use TestLib;
-use Test::More tests => 32;
+use Test::More tests => 46;

I think it might be better to move these testcases create a separate perl file.

About  0003
7) 
The v22-0003 seems simple and can remove lots of code in patch v22-0001, so
maybe we can merge 0001 and 0003 into one patch ?

Best regards,
Hou zj

Re: Tablesync early exit

[Peter Smith]

Patch v2 is the same; it only needed re-basing to the latest HEAD.


Kind Regards,
Peter Smith.
Fujitsu Australia


v2-0001-Tablesync-early-exit.patch
Description: Binary data

Re: Added schema level support for publication.

[Greg Nancarrow]

On Fri, Aug 27, 2021 at 4:13 PM vignesh C  wrote:
>
> I have implemented this in the 0003 patch, I have kept it separate to
> reduce the testing effort and also it will be easier if someone
> disagrees with the syntax. I will merge it to the main patch later
> based on the feedback. Attached v22 patch has the changes for the
> same.

Just experimenting with the new syntax so far, and seeing some new
messages and docs, I have the following suggestions for improvements:

src/backend/commands/publicationcmds.c

(1)
BEFORE:
for table/for all tables in schema should be specified before the object
AFTER:
FOR TABLE / FOR ALL TABLES IN SCHEMA should be specified before the
table/schema name(s)

(2)
BEFORE:
Tables cannot be added, dropped or set on FOR ALL TABLES publications.
AFTER:
Tables cannot be added to, dropped from, or set on FOR ALL TABLES publications.

(3)
BEFORE:
Schemas cannot be added, dropped or set on FOR ALL TABLES publications.
AFTER:
Schemas cannot be added to, dropped from, or set on FOR ALL TABLES publications.


v22-0002

doc/src/sgml/ref/create_publication.sgml

(1)
BEFORE:
+   Create a publication that publishes all changes for users and departments
+   table and that publishes all changes for all the tables present in the
AFTER:
+   Create a publication that publishes all changes for tables "users" and
+   "departments" and that publishes all changes for all the tables
present in the


Regards,
Greg Nancarrow
Fujitsu Australia

Re: [PATCH] More docs on what to do and not do in extension code

[Craig Ringer]

On Tue, 29 Jun 2021 at 13:30, Craig Ringer 
wrote:

> Laurenz,
>
> Thanks for your comments. Sorry it's taken me so long to get back to you.
> Commenting inline below on anything I think needs comment; other proposed
> changes look good.
>

I'm not going to get back to this anytime soon.

If anybody wants to pick it up that's great. Otherwise at least it's there
in the mailing lists to search.

Re: row filtering for logical replication

[Peter Smith]

On Fri, Aug 27, 2021 at 8:01 AM Peter Smith  wrote:
>
> On Thu, Aug 26, 2021 at 9:13 PM Amit Kapila  wrote:
> >
> > On Thu, Aug 26, 2021 at 3:41 PM Peter Smith  wrote:
> > >
> > > On Thu, Aug 26, 2021 at 3:00 PM Amit Kapila  
> > > wrote:
> > > >
> > > > On Thu, Aug 26, 2021 at 9:51 AM Peter Smith  
> > > > wrote:
> > > > >
> > > > > On Thu, Aug 26, 2021 at 1:20 PM Amit Kapila  
> > > > > wrote:
> > > > > >
> > > > > > On Thu, Aug 26, 2021 at 7:37 AM Peter Smith  
> > > > > > wrote:
> > > > > > >
> > > > > > > On Wed, Aug 25, 2021 at 3:28 PM Amit Kapila 
> > > > > > >  wrote:
> > > > > > > >
> > > > > > > ...
> > > > > > > >
> > > > > > > > Hmm, I think the gain via caching is not visible because we are 
> > > > > > > > using
> > > > > > > > simple expressions. It will be visible when we use somewhat 
> > > > > > > > complex
> > > > > > > > expressions where expression evaluation cost is significant.
> > > > > > > > Similarly, the impact of this change will magnify and it will 
> > > > > > > > also be
> > > > > > > > visible when a publication has many tables. Apart from 
> > > > > > > > performance,
> > > > > > > > this change is logically correct as well because it would be 
> > > > > > > > any way
> > > > > > > > better if we don't invalidate the cached expressions unless 
> > > > > > > > required.
> > > > > > >
> > > > > > > Please tell me what is your idea of a "complex" row filter 
> > > > > > > expression.
> > > > > > > Do you just mean a filter that has multiple AND conditions in it? 
> > > > > > > I
> > > > > > > don't really know if few complex expressions would amount to any
> > > > > > > significant evaluation costs, so I would like to run some timing 
> > > > > > > tests
> > > > > > > with some real examples to see the results.
> > > > > > >
> > > > > >
> > > > > > I think this means you didn't even understand or are convinced why 
> > > > > > the
> > > > > > patch has cache in the first place. As per your theory, even if we
> > > > > > didn't have cache, it won't matter but that is not true otherwise, 
> > > > > > the
> > > > > > patch wouldn't have it.
> > > > >
> > > > > I have never said there should be no caching. On the contrary, my
> > > > > performance test results [1] already confirmed that caching ExprState
> > > > > is of benefit for the millions of times it may be used in the
> > > > > pgoutput_row_filter function. My only doubts are in regard to how much
> > > > > observable impact there would be re-evaluating the filter expression
> > > > > just a few extra times by the get_rel_sync_entry function.
> > > > >
> > > >
> > > > I think it depends but why in the first place do you want to allow
> > > > re-evaluation when there is a way for not doing that?
> > >
> > > Because the current code logic of having the "delayed" ExprState
> > > evaluation does come at some cost.
> > >
> >
> > So, now you mixed it with the second point. Here, I was talking about
> > the need for correct invalidation but you started discussing when to
> > first time evaluate the expression, both are different things.
> >
> > >  And the cost is -
> > > a. Needing an extra condition and more code in the function 
> > > pgoutput_row_filter
> > > b. Needing to maintain the additional Node list
> > >
> >
> > I am not sure you need (b) above and I think (a) should make the
> > overall code look clean.
> >
> > > If we chose not to implement a delayed ExprState cache evaluation then
> > > there would still be a (one-time) ExprState cache evaluation but it
> > > would happen whenever get_rel_sync_entry is called (regardless of if
> > > pgoputput_row_filter is subsequently called). E.g. there can be some
> > > rebuilds of the ExprState cache if the user calls TRUNCATE.
> > >
> >
> > Apart from Truncate, it will also be a waste if any error happens
> > before actually evaluating the filter, tomorrow there could be other
> > operations like replication of sequences (I have checked that proposed
> > patch for sequences uses get_rel_sync_entry) where we don't need to
> > build ExprState (as filters might or might not be there). So, it would
> > be better to avoid cache lookups in those cases if possible. I still
> > think doing expensive things like preparing expressions should ideally
> > be done only when it is required.
>
> OK. Per your suggestion, I will try to move as much of the row-filter
> cache code as possible out of the get_rel_sync_entry function and into
> the pgoutput_row_filter function.
>

Here are the new v26* patches. This is a refactoring of the row-filter
caches to remove all the logic from the get_rel_sync_entry function
and delay it until if/when needed in the pgoutput_row_filter function.
This is now implemented per Amit's suggestion to move all the cache
code [1]. It is a replacement for the v25* patches.

The make check and TAP subscription tests are all OK. I have repeated
the performance tests [2] and those results are good too.

v26-0001 <--- v23 (base RF patch)
v26-0002 <--- ExprState cache mods 

Re: Remove Value node struct

[Kyotaro Horiguchi]

Agree to the motive and +1 for the concept.

At Wed, 25 Aug 2021 15:00:13 +0100, Dagfinn Ilmari Mannsåker 
 wrote in 
> However, the patch adds:
> 
> > +typedef struct Null
> > +{
> > +   NodeTag type;
> > +   char   *val;
> > +} Null;
> 
> which doesn't seem to be used anywhere. Is that a leftoverf from an
> intermediate development stage?

+1 Looks like so, it can be simply removed.

0001 looks fine to me.

0002:
  there's an "integer Value node" in gram.y: 7776.

-   n = makeFloatConst(v->val.str, location);
+   n = (Node *) makeFloatConst(castNode(Float, v)->val, 
location);

makeFloatConst is Node* so the cast doesn't seem needed. The same can
be said for Int and String Consts.  This looks like a confustion with
makeInteger and friends.

+   else if (IsA(obj, Integer))
+   _outInteger(str, (Integer *) obj);
+   else if (IsA(obj, Float))
+   _outFloat(str, (Float *) obj);

I felt that the type enames are a bit confusing as they might be too
generic, or too close with the corresponding binary types.


-   Node   *arg;/* a (Value *) or a (TypeName 
*) */
+   Node   *arg;

Mmm. It's a bit pity that we lose the generic name for the value nodes.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

Re: Separate out FileSet from SharedFileSet (was Re: pgsql: pgstat: Bring up pgstat in BaseInit() to fix uninitialized use o)

[Masahiko Sawada]

On Fri, Aug 27, 2021 at 2:25 PM Dilip Kumar  wrote:
>
> On Thu, Aug 26, 2021 at 2:10 PM Amit Kapila  wrote:
>>
>>
>> Thanks, Sawada-San and Dilip for confirmation. I would like to commit
>> this and the second patch (the second one still needs some more
>> testing and review) for PG-15 as there is no bug per-se related to
>> this work in PG-14 but I see an argument to commit this for PG-14 to
>> keep the code (APIs) consistent. What do you think? Does anybody else
>> have any opinion on this?
>>
>
> IMHO, this is a fair amount of refactoring and this is actually an 
> improvement patch so we should push it to v15.

I think so too.

Regards,

--
Masahiko Sawada
EDB:  https://www.enterprisedb.com/

Re: [PATCH] Tab completion for ALTER TABLE … ADD …

[Michael Paquier]

On Fri, Aug 27, 2021 at 11:52:33AM +0100, Dagfinn Ilmari Mannsåker wrote:
> That was easy enough to add (just a bit of extra fiddling to handle
> COLUMN being optional), done in the attached v2 patch.

This part was a bit misleading, as it would recommend a list of types
when specifying just ADD CONSTRAINT for example, so I have removed
it.  An extra thing that felt a bit overdoing is the addition of KEY
after PRIMARY/FOREIGN.

> Doing a list of arbitrarily many comma-separated names is more
> complicated, so that can be the subject for another patch.

No objections to that.  I have applied what we have now, as that's
already an improvement.
--
Michael


signature.asc
Description: PGP signature

Re: Write visibility map during CLUSTER/VACUUM FULL

[Justin Pryzby]

On Mon, Jun 28, 2021 at 11:22:01AM +0300, Anna Akenteva wrote:
> On 2019-11-29 05:32, Michael Paquier wrote:
> > These comments are unanswered for more than 2 months, so I am marking
> > this entry as returned with feedback.
> 
> I'd like to revive this patch. To make further work easier, attaching a
> rebased version of the latest patch by Alexander.
> 
> As for having yet another copy of logic determining visibility: the
> visibility check was mainly taken from heap_page_is_all_visible(), so I
> refactored the code to make sure that logic is not duplicated. The updated
> patch is attached too.

I agree that it's strange that VACUUM(FREEZE) freezes tuples but not VM bits,
nor page-level PD_ALL_VISIBLE (which is implied by all frozen).  After FULL
runs (taking an exclusive lock on the table), it's necessary to then vacuum the
table again to get what's intended.

Rebased on f10f0ae420ee62400876ab34dca2c09c20dcd030.

And rephrased Anna's two independent/alternate patches as a 2nd patch on top of
the 1st, as that helps me to read it and reduces its total size.

I noticed in testing the patch that autovacuum is still hitting the relation
shortly after vacuum full.  This is due to n_ins_since_autovacuum, which is new
in pg13.  I don't know how to address that (or even if it should be addressed
at all).

Also, pg_class.relallvisible is not set unless vacuum/analyze is run again
(which is mitigated by the n_ins behavior above).  It seems like this might be
important: an plan which uses index-only scan might be missed in favour of
something else until autovacuum runs (it will use cost-based delay, and might
not be scheduled immediately, could be interrupted, or even diasbled).

I'm testing like this:
CREATE EXTENSION IF NOT EXISTS pg_visibility ; DROP TABLE t; CREATE TABLE t AS 
SELECT generate_series(1,9); VACUUM FULL t; ANALYZE t; SELECT 
n_ins_since_vacuum, n_tup_upd, n_tup_del, n_tup_hot_upd FROM pg_stat_all_tables 
WHERE relname='t'; SELECT relpages, relallvisible FROM pg_class WHERE 
oid='t'::regclass; SELECT COUNT(1), COUNT(1)FILTER(WHERE all_visible='t') 
allvis, COUNT(1)FILTER(WHERE all_frozen='t') allfrz, COUNT(1)FILTER(WHERE 
pd_all_visible='t') allvispd FROM pg_visibility('t');

-- 
Justin
>From 4ec10accb276d9ecde461c6cde0be5a610ed3dc6 Mon Sep 17 00:00:00 2001
From: Justin Pryzby 
Date: Sun, 29 Aug 2021 15:57:09 -0500
Subject: [PATCH 1/3] VACUUM FULL/CLUSTER: set VM and page-level visibility
 bits

From: 	Alexander Korotkov
write-vm-during-cluster-3-rebase
---
 src/backend/access/heap/rewriteheap.c   | 131 +++-
 src/backend/access/heap/visibilitymap.c |  18 
 src/include/access/visibilitymap.h  |  18 
 3 files changed, 147 insertions(+), 20 deletions(-)

diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c
index 986a776bbd..af0d1b2d0c 100644
--- a/src/backend/access/heap/rewriteheap.c
+++ b/src/backend/access/heap/rewriteheap.c
@@ -110,6 +110,7 @@
 #include "access/heaptoast.h"
 #include "access/rewriteheap.h"
 #include "access/transam.h"
+#include "access/visibilitymap.h"
 #include "access/xact.h"
 #include "access/xloginsert.h"
 #include "catalog/catalog.h"
@@ -152,6 +153,11 @@ typedef struct RewriteStateData
 	HTAB	   *rs_old_new_tid_map; /* unmatched B tuples */
 	HTAB	   *rs_logical_mappings;	/* logical remapping files */
 	uint32		rs_num_rewrite_mappings;	/* # in memory mappings */
+	Page		rs_vm_buffer;	/* visibility map page */
+	BlockNumber rs_vm_blockno;	/* block number of visibility page */
+	BlockNumber rs_vm_buffer_valid;	/* T if any bits are set */
+	bool		rs_all_visible;	/* all visible flag for rs_buffer */
+	bool		rs_all_frozen;	/* all frozen flag for rs_buffer */
 }			RewriteStateData;
 
 /*
@@ -213,6 +219,9 @@ typedef struct RewriteMappingDataEntry
 
 
 /* prototypes for internal functions */
+static void rewrite_flush_vm_page(RewriteState state);
+static void rewrite_set_vm_flags(RewriteState state);
+static void rewrite_update_vm_flags(RewriteState state, HeapTuple tuple);
 static void raw_heap_insert(RewriteState state, HeapTuple tup);
 
 /* internal logical remapping prototypes */
@@ -264,6 +273,11 @@ begin_heap_rewrite(Relation old_heap, Relation new_heap, TransactionId oldest_xm
 	state->rs_freeze_xid = freeze_xid;
 	state->rs_cutoff_multi = cutoff_multi;
 	state->rs_cxt = rw_cxt;
+	state->rs_vm_buffer = (Page) palloc(BLCKSZ);
+	state->rs_vm_blockno = HEAPBLK_TO_MAPBLOCK(state->rs_blockno);
+	state->rs_vm_buffer_valid = false;
+	state->rs_all_visible = true;
+	state->rs_all_frozen = true;
 
 	/* Initialize hash tables used to track update chains */
 	hash_ctl.keysize = sizeof(TidHashKey);
@@ -284,6 +298,9 @@ begin_heap_rewrite(Relation old_heap, Relation new_heap, TransactionId oldest_xm
 	&hash_ctl,
 	HASH_ELEM | HASH_BLOBS | HASH_CONTEXT);
 
+	if (!smgrexists(RelationGetSmgr(state->rs_new_rel), VISIBILITYMAP_FORKNUM))
+		smgrcreate(RelationGetSmgr(state->rs_new_rel), VISIBILITYMAP_FORKNUM, false);

jff: checksum algorithm is not as intended

[Yura Sokolov]

Good day.

Current checksum is not calculated in intended way and
has the flaw.

Single round function is written as:

#define CHECKSUM_COMP(checksum, value) do {\
uint32 __tmp = (checksum) ^ (value);\
(checksum) = __tmp * FNV_PRIME ^ (__tmp >> 17);\
} while (0)

And looks like it was intended to be
(checksum) = (__tmp * FNV_PRIME) ^ (__tmp >> 17);

At least original Florian Pflug suggestion were correctly written
in this way (but with shift 1):
https://www.postgresql.org/message-id/99343716-5F5A-45C8-B2F6-74B9BA357396%40phlo.org

But due to C operation precedence it is actually calculated as:
(checksum) = __tmp * (FNV_PRIME ^ (__tmp >> 17));

It has more longer collision chains and worse: it has 256 pathological
result slots of shape 0xXX00 each has 517 collisions in average.
Totally 132352 __tmp values are collided into this 256 slots.

That is happens due to if top 16 bits are happens to be
0x0326 or 0x0327, then `FNV_PRIME ^ (__tmp >> 17) == 0x100`,
and then `__tmp * 0x100` keeps only lower 8 bits. That means,
9 bits masked by 0x0001ff00 are completely lost.

mix(0x03260001) == mix(0x03260101) = mix(0x0327aa01) == mix(0x0327ff01)
(where mix is a `__tmp` to `checksum` transformation)

regards,
Yura Sokolov
y.soko...@postgrespro.ru
funny.fal...@gmail.com

PS. Test program in Crystal language is attached and output for current
CHECKSUM_COMP implementation and "correct" (intended).
Excuse me for Crystal, it is prettier to write for small compiled 
scripts.- 17 items at 38 slots
- 16 items at 114 slots
- 15 items at 456 slots
- 14 items at 1690 slots
- 13 items at 3706 slots
- 12 items at 11128 slots
- 11 items at 30994 slots
- 10 items at 84940 slots
- 9 items at 259160 slots
- 8 items at 778102 slots
- 7 items at 2413624 slots
- 6 items at 8670652 slots
- 5 items at 27772912 slots
- 4 items at 93426142 slots
- 3 items at 288544882 slots
- 2 items at 752648630 slots
- 1 items at 1332584708 slots
- 0 items at 1787735418 slots
Pathological 513 collisions at slot 
Pathological 515 collisions at slot 0100
Pathological 517 collisions at slot 0200
Pathological 518 collisions at slot 0300
Pathological 516 collisions at slot 0400
Pathological 514 collisions at slot 0500
Pathological 516 collisions at slot 0600
Pathological 516 collisions at slot 0700
Pathological 515 collisions at slot 0800
Pathological 518 collisions at slot 0900
Pathological 516 collisions at slot 0a00
Pathological 516 collisions at slot 0b00
Pathological 517 collisions at slot 0c00
Pathological 515 collisions at slot 0d00
Pathological 516 collisions at slot 0e00
Pathological 514 collisions at slot 0f00
Pathological 515 collisions at slot 1000
Pathological 516 collisions at slot 1100
Pathological 515 collisions at slot 1200
Pathological 518 collisions at slot 1300
Pathological 514 collisions at slot 1400
Pathological 516 collisions at slot 1500
Pathological 515 collisions at slot 1600
Pathological 516 collisions at slot 1700
Pathological 515 collisions at slot 1800
Pathological 516 collisions at slot 1900
Pathological 515 collisions at slot 1a00
Pathological 519 collisions at slot 1b00
Pathological 516 collisions at slot 1c00
Pathological 517 collisions at slot 1d00
Pathological 515 collisions at slot 1e00
Pathological 517 collisions at slot 1f00
Pathological 514 collisions at slot 2000
Pathological 517 collisions at slot 2100
Pathological 517 collisions at slot 2200
Pathological 518 collisions at slot 2300
Pathological 515 collisions at slot 2400
Pathological 517 collisions at slot 2500
Pathological 519 collisions at slot 2600
Pathological 516 collisions at slot 2700
Pathological 514 collisions at slot 2800
Pathological 518 collisions at slot 2900
Pathological 515 collisions at slot 2a00
Pathological 516 collisions at slot 2b00
Pathological 517 collisions at slot 2c00
Pathological 514 collisions at slot 2d00
Pathological 516 collisions at slot 2e00
Pathological 517 collisions at slot 2f00
Pathological 515 collisions at slot 3000
Pathological 517 collisions at slot 3100
Pathological 518 collisions at slot 3200
Pathological 519 collisions at slot 3300
Pathological 515 collisions at slot 3400
Pathological 519 collisions at slot 3500
Pathological 517 collisions at slot 3600
Pathological 517 collisions at slot 3700
Pathological 516 collisions at slot 3800
Pathological 516 collisions at slot 3900
Pathological 519 collisions at slot 3a00
Pathological 517 collisions at slot 3b00
Pathological 517 collisions at slot 3c00
Pathological 516 collisions at slot 3d00
Pathological 517 collisions at slot 3e00
Pathological 518 collisions at slot 3f00
Pathological 514 collisions at slot 4000
Pathological 518 collisions at slot 4100
Pathological 515 collisions at slot 42000

Re: Schema variables - new implementation for Postgres 15

[Pavel Stehule]

so 28. 8. 2021 v 11:57 odesílatel Gilles Darold  napsal:

> Hi,
>
> Review resume:
>
>
> This patch implements Schema Variables that are database objects that can
> hold a single or composite value following the data type used at variable
> declaration. Schema variables, like relations, exist within a schema and
> their access is controlled via GRANT and REVOKE commands. The schema
> variable can be created by the CREATE VARIABLE command, altered using ALTER
> VARIABLE and removed using DROP VARIABLE.
>
> The value of a schema variable is local to the current session. Retrieving
> a variable's value returns either a NULL or a default value, unless its
> value is set to something else in the current session with a LET command.
> The content of a variable is not transactional. This is the same as in
> regular variables in PL languages.
>
> Schema variables are retrieved by the SELECT SQL command. Their value is
> set with the LET SQL command. While schema variables share properties with
> tables, their value cannot be updated with an UPDATE command.
>
> The patch apply with the patch command without problem and compilation
> reports no warning or errors. Regression tests pass successfully using make
> check or make installcheck
> It also includes all documentation and regression tests.
>
> Performances are near the set of plpgsql variable settings which is
> impressive:
>
> do $$
> declare var1 int ; i int;
> begin
>   for i in 1..100
>   loop
> var1 := i;
>   end loop;
> end;
> $$;
> DO
> Time: 71,515 ms
>
> CREATE VARIABLE var1 AS integer;
> do $$
> declare i int ;
> begin
>   for i in 1..100
>   loop
> let var1 = i;
>   end loop;
> end;
> $$;
> DO
> Time: 94,658 ms
>
> There is just one thing that puzzles me. We can use :
>
> CREATE VARIABLE var1 AS date NOT NULL;
> postgres=# SELECT var1;
> ERROR:  null value is not allowed for NOT NULL schema variable "var1"
>
> which I understand and is the right behavior. But if we use:
>
> CREATE IMMUTABLE VARIABLE var1 AS date NOT NULL;
> postgres=# SELECT var1;
> ERROR:  null value is not allowed for NOT NULL schema variable "var1"
> DETAIL:  The schema variable was not initialized yet.
> postgres=# LET var1=current_date;
> ERROR:  schema variable "var1" is declared IMMUTABLE
>
> It should probably be better to not allow NOT NULL when IMMUTABLE is used
> because the variable can not be used at all.  Also probably IMMUTABLE
> without a DEFAULT value should also be restricted as it makes no sens. If
> the user wants the variable to be NULL he must use DEFAULT NULL. This is
> just a though, the above error messages are explicit and the user can
> understand what wrong declaration he have done.
>

I thought about this case, and I have one scenario, where this behaviour
can be useful. When the variable is declared as IMMUTABLE NOT NULL without
not null default, then any access to the content of the variable has to
fail. I think it can be used for detection, where and when the variable is
first used. So this behavior is allowed just because I think, so this
feature can be interesting for debugging. If this idea is too strange, I
have no problem to disable this case.

Regards

Pavel


>
> Except that I think this patch is ready for committers, so if there is no
> other opinion in favor of restricting the use of IMMUTABLE with NOT NULL
> and DEFAULT I will change the status to ready for committers.
>
> --
> Gilles Daroldhttp://www.darold.net/
>
>

Re: FYA: VITAL INFO

[Osahon Oduware]

LEAH  SHARIBU: News making round states that she escaped early Wednesday
morning (August 18, 2021), from where she was held hostage around B.I.U.,
Ugbor, Benin City, Edo State, Nigeria, after being released by her Islamic
captors some months back. Some influential persons were involved in her
being held hostage.
=
Criminals hiding as patients in & around the Neuro Psychiatric Hospital,
Idowina, Benin City, Edo State, Nigeria.
==
C RIME AGAINST HUMANITY: Farm Lands majorly belonging to top military
officers & influential persons allegedly being used across Nigeria for
organized crime (kidnap et al), burying of crime victims & hiding deadly
weapons (usually beneath the ground camouflaged as food crops). #Terrorism

A CALL TO INVESTIGATE MR. ROLAND KLAUS FOR ACTS OF TERRORISM IN NIGERIA
By Osahon Godwin ODUWARE
Source-Link: https://usaupload.com/file/JqV/Roland_Klaus_Terrorism.pdf>

  **Copy links to your browser address bar IF not displaying upon click:

***
I would want Mr. Roland Klaus (C.E.O. GIS Transport Nigeria Ltd.), who has
close ties with the Nigerian & German Military, investigated for ACTS OF
TERRORISM in NIGERIA. As well as others involved in criminal activities
mentioned in related doc links below (Car explosion, Armed Robbery, Kidnap,
etc):
**NB: It should NOT be surprising why Osama Bin Laden could NOT be easily
located as he was given cover by the Pakistani Military until he was
discovered in one their bases during a secret mission.**
- Putting Things Straight:
https://usaupload.com/FR4/Putting_Things_Straight.pdf
- Birthday Reflection, Car Explosion + More:
https://usaupload.com/file/FR3/SHARE_Birthday-Reflection_Car-Explosion_Putting-Things-Straight.jpg
- Racist Email:
https://www.docdroid.net/uftX2s0/racistmail.pdf
- Provocative Message & Dismissal Clarification:
https://usaupload.com/IVi/07B_Provocative_Message_Dismissal_Clarification.pdf

- Employment Letter:
https://www.docdroid.net/AgknQAT/godwin-employment-letter-acknoledged-2020-02-01-pdf

- Termination Letter:
https://www.docdroid.net/nc7iTa8/termination-letter-pdf
- Employment/Termination Letter Request Mail (Showing disorganization in the
office & no stated policy as claimed that I violated):
https://www.docdroid.net/iBCr1IK/employment-letter-request-mail-old-pdf
- Resume:
https://usaupload.com/IS9/CV_Professional_Certs.pdf
- Docs.zip:
https://usaupload.com/file/Fni/DOCS.zip
- Docs.zip Content:
https://usaupload.com/file/FR7/__Google_Drive_Docs_Folder_Links_Content.pdf

***
SHAME!!!
>From OSAHON G. ODUWARE: "Got a call today, Monday, July 26, 2021, on my
line (+2347060408906) from the Dept. of Homeland Office, routed through
their West-Africa base in Senegal (+221773339425) & a senior staff
(Supervisor) stated that he can't access the links in the doc below due to
Cyber-Security crap, then how will they handle links relating to
crime/emergency":
Vital Info Links:
https://usaupload.com/file/GtE/Vital_Info_Links.pdf

NB: The "American Dream" Obama presented to the world was actually a
NIGHTMARE, not any close to the dream of the Founding Fathers.
***
SECURITY ALERT:
https://usaupload.com/FR2/Security_Alert.pdf

***
[ADDITIONAL INFO]
ISLAM-MUHAMMAD:
https://usaupload.com/file/FR6/05_Islam_Muhammad.pdf


I Can’t Believe Rumours Leah Sharibu Converted To Islam, Has Babies Until I
See Her – Mother
<
https://punchng.com/i-cant-believe-rumours-leah-sharibu-converted-to-islam-has-babies-until-i-see-her-mother/
>
Rather unfortunately, this defines the Islam Religion I have come to know
from acclaimed followers of Prophet Muhammad who himself lived a similar
lifestyle - Forceful conversion (Muhammad did same to several Jews during
his time), Forceful marriage (Muhammad did same with some ladies he
married), Falsehood/Pretense, Unlawful Killings, etc.
It should NOT be surprising anymore that most OCCULT GROUPS given cover by
the SECRET SERVICE globally (as agents) indulge in similar practices.

**
BEWARE OF ATHEISTS!!!
Atheists do NOT believe in the existence of God (the creator of the Heavens
and Earth).
In Romans 1:18-32, the Bible reveals an alarming nature of people in this
category who have NO regard for God and history/statistics confirms this so.
I stand to be corrected, I have come to understand from my experiences, as
well as  that of others, that those who do NOT believe in God (regardless
of the religion they identify with) have NO value for human lives and
constitute the major challenges facing the world today (murder, kidnap,
rape,  and several acts of terrorism).

**

"...Great heart-felt appreciation goes to the likes of Aunty Gail, ORUEF
team, other foreigners who made selfless sacrifices to come from America
and other foreign nations to Nigeria to assist/particip

Re: Spelling change in LLVM 14 API

[Andres Freund]

Hi,

On 2021-08-22 09:22:43 -0400, Tom Lane wrote:
> Seems like either we should push back on pointless renaming, or else
> that we're not really supposed to be accessing this non-stable API.

Unfortunately LLVM only considers the C API (and even there only subsets) as
stable. Most of our code uses the stable C API, but there are parts where that
wasn't / isn't sufficient.

The weirdest part about this change [1] is that it's doing such a halfway
job. There's plenty other variants of the hasFnAttribute() spelling around,
e.g. in Function.h ([2]). I kinda get wanting to clean up that half the
functions are named like hasFnAttr() and the other like hasFnAttribute(), but
right now it seems to make things worse rather than better.

Right now the easiest change would be to just use the
Function::hasFnAttribute() helper function. But I'm a bit loathe to do so
because in a way one would hope that that gets changed too :(.

The next best thing seems to be to use the slightly longer form spelling, like
this:

diff --git i/src/backend/jit/llvm/llvmjit_inline.cpp 
w/src/backend/jit/llvm/llvmjit_inline.cpp
index 6f03595db5a..eb350003935 100644
--- i/src/backend/jit/llvm/llvmjit_inline.cpp
+++ w/src/backend/jit/llvm/llvmjit_inline.cpp
@@ -594,7 +594,7 @@ function_inlinable(llvm::Function &F,
if (F.materialize())
elog(FATAL, "failed to materialize metadata");
 
-   if (F.getAttributes().hasFnAttribute(llvm::Attribute::NoInline))
+   if (F.getAttributes().hasAttribute(llvm::Attribute::FunctionIndex, 
llvm::Attribute::NoInline))
{
ilog(DEBUG1, "ineligibile to import %s due to noinline",
 F.getName().data());

which seems not likely to fall under the same "cleanup" spree.


On 2021-08-29 12:47:38 -0400, Alvaro Herrera wrote:
> On 2021-Aug-29, Tom Lane wrote:
> > I stand by my opinion that Thomas' patch is a kluge rather than something
> > we should accept as a long-term answer.  However, in the interests of
> > keeping the buildfarm green, maybe we should commit it until we have a
> > better solution.  It looks like seawasp is only building HEAD, so I think
> > we could commit this in HEAD only.
> 
> Well, I definitely agree with your opinion, but perhaps we should
> consider what to do in case LLVM developers decide not to care and keep
> the rename.

Yea :(.

I just pinged them, but I don't have much hope that this will be backed out at
this point. There's probably more llvm users that already adapted their code
than "us".


> I'm not sure that polluting the tree with kludges for
> cross-LLVM-version compatibility is the best way to handle this kind of
> thing.  Maybe it'd be better to try and centralize them in a single file
> perhaps.

I think that makes sense for things that are in more than one place, but if
there's just a single case of the ifdef it doesn't seem that obvious to me. In
particular because there's a C / C++ boundary involved here...

Greetings,

Andres Freund


[1] 
https://github.com/llvm/llvm-project/commit/92ce6db9ee7666a347fccf0f72ba3225b199d6d1
[2] 
https://github.com/llvm/llvm-project/blob/96d329455501dd9f7b38c6f4c5d54c7e13247dd1/llvm/include/llvm/IR/Function.h#L390

Re: Spelling change in LLVM 14 API

[Alvaro Herrera]

On 2021-Aug-29, Tom Lane wrote:

> I stand by my opinion that Thomas' patch is a kluge rather than something
> we should accept as a long-term answer.  However, in the interests of
> keeping the buildfarm green, maybe we should commit it until we have a
> better solution.  It looks like seawasp is only building HEAD, so I think
> we could commit this in HEAD only.

Well, I definitely agree with your opinion, but perhaps we should
consider what to do in case LLVM developers decide not to care and keep
the rename.  I'm not sure that polluting the tree with kludges for
cross-LLVM-version compatibility is the best way to handle this kind of
thing.  Maybe it'd be better to try and centralize them in a single file
perhaps.  For example, pglogical has files for each PG version it
supports so that the main source code only has to use one wording of
each call:

https://github.com/2ndQuadrant/pglogical/blob/REL2_x_STABLE/compat14/pglogical_compat.h
https://github.com/2ndQuadrant/pglogical/blob/REL2_x_STABLE/compat96/pglogical_compat.c

-- 
Álvaro Herrera PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"La rebeldía es la virtud original del hombre" (Arthur Schopenhauer)

Re: inconsistent behavior with "GENERATED BY DEFAULT AS IDENTITY"

[Himanshu Upadhyaya]

ok, understood.

Thanks Tom.

Regards,
Himanshu

On Sun, Aug 29, 2021 at 7:10 PM Tom Lane  wrote:

> Himanshu Upadhyaya  writes:
> > IMHO below query should replace "NULL" value for ID column with the
> > GENERATED IDENTITY value (should insert 1,10 for ID and ID1 respectively
> in
> > below's example), similar to what we expect when we have DEFAULT
> constraint
> > on the column.
>
> Why?  Ordinary DEFAULT clauses do not act that way; if you specify NULL
> (or any other value) that is what you get.  If you want the default
> value, you can omit the column, or write DEFAULT.
>
> > Any reason for disallowing NULL insertion?
>
> Consistency and standards compliance.
>
> regards, tom lane
>

Re: Spelling change in LLVM 14 API

[Tom Lane]

Alvaro Herrera  writes:
> On 2021-Aug-22, Tom Lane wrote:
>> Thomas Munro  writes:
>>> After [1], seawasp blew up[2].  I tested the following fix on LLVM 13
>>> and 14 (main branch ~2 days ago).  Better ideas welcome.
>>> 
>>> -   if (F.getAttributes().hasFnAttribute(llvm::Attribute::NoInline))
>>> +#if LLVM_VERSION_MAJOR < 14
>>> +#define hasFnAttr hasFnAttribute
>>> +#endif
>>> +
>>> +   if (F.getAttributes().hasFnAttr(llvm::Attribute::NoInline))

>> Seems like either we should push back on pointless renaming, or else
>> that we're not really supposed to be accessing this non-stable API.
>> I have no idea which of those situations applies ... but if the LLVM
>> guys are randomly renaming methods that *are* supposed to be
>> user-visible, they need re-education.

> Did anything happen?  Seawasp is still red ...

I stand by my opinion that Thomas' patch is a kluge rather than something
we should accept as a long-term answer.  However, in the interests of
keeping the buildfarm green, maybe we should commit it until we have a
better solution.  It looks like seawasp is only building HEAD, so I think
we could commit this in HEAD only.

regards, tom lane

Re: AIX: Symbols are missing in libpq.a

[Noah Misch]

On Thu, Aug 26, 2021 at 12:49:01PM +, REIX, Tony wrote:
> While porting postgresql-odbc v13 to AIX, we have found that (at least) 2 
> symbols are missing in libpq.a provided by the port of PostgreSQL v13.1 to 
> AIX 7.1 by the BullFreeware project :
> 
> pg_char_to_encoding
> pg_encoding_to_char
> 
> Looking at details, it appears that these symbols are present in version 12.8 
> .
> They were still missing in 13.4 .
> Something has changed between v12 and v13.
> 
> Looking at more details, the way libpq.a is built on AIX is different from 
> the way libpq.so is built on Linux.
> On Linux, the file "exports.txt" is used for building the list of symbols to 
> be exported.
> On AIX, the tool  mkldexport.sh is used for dynamically generating the 
> symbols to be exported.
> And it appears that 5 symbols (including the 2 above) are missing on AIX. 
> Don't know why.

Would you study why it changed?  If $(MKLDEXPORT) is no longer able to find
all symbols, then we're likely to have problems in more libraries than libpq,
including libraries that don't use a $(SHLIB_EXPORTS) file.

Re: Spelling change in LLVM 14 API

[Alvaro Herrera]

On 2021-Aug-22, Tom Lane wrote:

> Thomas Munro  writes:
> > After [1], seawasp blew up[2].  I tested the following fix on LLVM 13
> > and 14 (main branch ~2 days ago).  Better ideas welcome.
> 
> > -   if (F.getAttributes().hasFnAttribute(llvm::Attribute::NoInline))
> > +#if LLVM_VERSION_MAJOR < 14
> > +#define hasFnAttr hasFnAttribute
> > +#endif
> > +
> > +   if (F.getAttributes().hasFnAttr(llvm::Attribute::NoInline))
> 
> Seems like either we should push back on pointless renaming, or else
> that we're not really supposed to be accessing this non-stable API.
> I have no idea which of those situations applies ... but if the LLVM
> guys are randomly renaming methods that *are* supposed to be
> user-visible, they need re-education.

Did anything happen?  Seawasp is still red ...

-- 
Álvaro Herrera

Re: inconsistent behavior with "GENERATED BY DEFAULT AS IDENTITY"

[Tom Lane]

Himanshu Upadhyaya  writes:
> IMHO below query should replace "NULL" value for ID column with the
> GENERATED IDENTITY value (should insert 1,10 for ID and ID1 respectively in
> below's example), similar to what we expect when we have DEFAULT constraint
> on the column.

Why?  Ordinary DEFAULT clauses do not act that way; if you specify NULL
(or any other value) that is what you get.  If you want the default
value, you can omit the column, or write DEFAULT.

> Any reason for disallowing NULL insertion?

Consistency and standards compliance.

regards, tom lane

inconsistent behavior with "GENERATED BY DEFAULT AS IDENTITY"

[Himanshu Upadhyaya]

Hi,

It seems we have inconsistent behavior with the implementation of
"GENERATED BY DEFAULT AS IDENTITY" constraint on a table column.
Here we are not allowing(internally not replacing NULL with IDENTITY
DEFAULT) the "NULL" insertion into the table column.

postgres=# CREATE TABLE TEST_TBL_1(ID INTEGER  GENERATED BY DEFAULT AS
IDENTITY ,ID1 INTEGER);
CREATE TABLE
postgres=# insert into TEST_TBL_1 values  (NULL, 10);
ERROR:  null value in column "id" of relation "test_tbl_1" violates
not-null constraint
DETAIL:  Failing row contains (null, 10).
postgres=# insert into TEST_TBL_1(id1) values  ( 10);
INSERT 0 1


However this is allowed on normal default column:
postgres=# create table TEST_TBL_2 (ID INTEGER  DEFAULT 10 ,ID1 INTEGER);
CREATE TABLE
postgres=# insert into TEST_TBL_2 values  (NULL, 10);
INSERT 0 1
postgres=# insert into TEST_TBL_2 (id1) values  (20);
INSERT 0 1


if I understand it correctly, the use-case for supporting "GENERATED BY
DEFAULT AS IDENTITY" is to have an inbuilt sequence generated DEFAULT value
for a column.

IMHO below query should replace "NULL" value for ID column with the
GENERATED IDENTITY value (should insert 1,10 for ID and ID1 respectively in
below's example), similar to what we expect when we have DEFAULT constraint
on the column.

insert into TEST_TBL_1 values  (NULL, 10);

TO Support the above query ORACLE is having "GENERATED BY DEFAULT ON NULL
AS IDENTITY" syntax. We can also think on similar lines and have similar
implementation
or allow it under "GENERATED BY DEFAULT AS IDENTITY" itself.

Any reason for disallowing NULL insertion?

Thoughts?

Thanks,
Himanshu