Thread: pg_get_expr() hangs with uncommitted DDL transaction

Hello,

I noticed the following behaviour in the JDBC driver:

In one transaction run an ALTER TABLE ADD COLUMN ... statement with auto-commit off, but don't commit the statement

In another transcation, call DatabaseMetaData.getColumns() for this table - this call will wait until the first
transactioncommits or rolls back. 

I could narrow this down to the usage of pg_catalog.pg_get_expr() in the statement that the JDBC driver uses.

A stripped down version of that statement is this:

    SELECT a.attname, .....
           pg_catalog.pg_get_expr(def.adbin, def.adrelid) AS adsrc -- << this is the "problem"
    FROM pg_catalog.pg_namespace n
      JOIN pg_catalog.pg_class c ON (c.relnamespace = n.oid)
      JOIN pg_catalog.pg_attribute a ON (a.attrelid=c.oid)
      LEFT JOIN pg_catalog.pg_attrdef def ON (a.attrelid=def.adrelid AND a.attnum = def.adnum)
    WHERE a.attnum > 0
     AND NOT a.attisdropped
     AND c.relname LIKE 'foo'
     AND n.nspname LIKE 'public';

When the call to pg_catalog.pg_get_expr() is removed from this statement, it will return even if the ALTER TABLE has
notbeen committed. 

Is there a reason why pg_catalog.pg_get_expr() will wait until the exclusive lock on the table is released?
The value from pg_attrdef.adbin can be selected without any problems, so it appears to be something inside the
function.

Versions used:

Postgres: PostgreSQL 9.3.5, compiled by Visual C++ build 1600, 64-bit
JDBC Driver: PostgreSQL 9.3 JDBC4 (build 1102)

Regards
Thomas

Thomas Kellerer <spam_eater@gmx.net> writes:
> Is there a reason why pg_catalog.pg_get_expr() will wait until the exclusive lock on the table is released?

Yes.  It needs to extract attribute names, data types, etc for the target
table and it would also like to be sure that that data is self-consistent.
So it takes out AccessShareLock the same as a query on the table would.

There have been periodic debates about how it would be nice if this and
related ruleutils.c functions would work based on the calling query's
snapshot instead of trying to provide up-to-date info.  However, short
of a rather massive rewrite (and, probably, a lot of duplicative code)
that's not going to happen :-(

            regards, tom lane

Tom Lane schrieb am 11.11.2014 um 16:35:
>> Is there a reason why pg_catalog.pg_get_expr() will wait until the exclusive lock on the table is released?
>
> Yes.  It needs to extract attribute names, data types, etc for the target
> table and it would also like to be sure that that data is self-consistent.
> So it takes out AccessShareLock the same as a query on the table would.

Not sure I understand this.

Extracting the attribute names and data types from the various pg_catalog tables
works _without_ the query being blocked. Even when selecting all arguments that
are passed to pg_get_expr().

It's only when adding the function call to an otherwise perfectly working query
that this wait occurs.

Does this mean that pg_get_expr() selects additional data from the table,
instead of just using the values that are provided by the caller?

Thomas

Thomas Kellerer <spam_eater@gmx.net> writes:
> Does this mean that pg_get_expr() selects additional data from the table,
> instead of just using the values that are provided by the caller?

No, it means it depends on backend code that is also used when accessing
the table "for real", and that code doesn't work safely without a lock.
(In particular, a lot of what ruleutils.c does ultimately reduces to
catalog cache lookups, and we can *not* allow stale data in those caches.)

In principle we could reimplement pg_get_expr and sibling routines as code
that just looks at the contents of the catalogs as they stood at the time
of the surrounding query's snapshot.  But that's not what they do today,
and getting from point A to point B would be a lot of work.

            regards, tom lane

Tom Lane schrieb am 11.11.2014 um 17:08:
>> Does this mean that pg_get_expr() selects additional data from the table,
>> instead of just using the values that are provided by the caller?
>
> No, it means it depends on backend code that is also used when accessing
> the table "for real", and that code doesn't work safely without a lock.
> (In particular, a lot of what ruleutils.c does ultimately reduces to
> catalog cache lookups, and we can *not* allow stale data in those caches.)

Ah! Understood. Thanks for the explanation.

Thomas