Thread: get/setReadOnly broken if default_transaction_read_only on

Hi,

I've found this in the git head so it applies to current versions.

AbstractJdbc2Connection initializes its readOnly member to a
hardcoded false instead of to 'show default_transaction_read_only'
from the backend.

That means two things: 1. getReadOnly() is wrong in case the
backend has default_transaction_read_only on, and

2. setReadOnly(false) doesn't work in that case, because the value
is compared to readOnly and looks the same, so no set session
characteristics command is sent to the backend.


An application can work around (2) by always calling
setReadOnly(true);setReadOnly(false); if it wants to write.
But that's a bit ugly.

The driver could do that too for a quick hack, but it would
be nice to query the correct value from the backend and use that.

Cheers,
Chapman Flack

J Chapman Flack <jflack@math.purdue.edu> wrote:

> AbstractJdbc2Connection initializes its readOnly member to a
> hardcoded false instead of to 'show default_transaction_read_only'
> from the backend.

> An application can work around (2) by always calling
> setReadOnly(true);setReadOnly(false); if it wants to write.
> But that's a bit ugly.
>
> The driver could do that too for a quick hack, but it would
> be nice to query the correct value from the backend and use that.

+1

This may become increasingly important as Serializable Snapshot
Isolation becomes more popular.  Read only transactions are
significantly more efficient than read-write transactions at the
serializable transaction isolation level in PostgreSQL version 9.1
or higher.  Since our shop has about 90% read only transactions in
common workloads, we have set some servers to
default_transaction_read_only = on in the configuration file and
only turn it off for those transactions which write (or might need
to write).  It would be nice to drop that workaround for the current
behavior.  It should probably do something similar to what's done
for transaction isolation level on connection.

-Kevin

On 06/07/2012 04:11 AM, J Chapman Flack wrote:
>
> Hi,
>
> I've found this in the git head so it applies to current versions.
>
> AbstractJdbc2Connection initializes its readOnly member to a
> hardcoded false instead of to 'show default_transaction_read_only'
> from the backend.
>
> That means two things: 1. getReadOnly() is wrong in case the
> backend has default_transaction_read_only on, and
>
> 2. setReadOnly(false) doesn't work in that case, because the value
> is compared to readOnly and looks the same, so no set session
> characteristics command is sent to the backend.
>
>
> An application can work around (2) by always calling
> setReadOnly(true);setReadOnly(false); if it wants to write.
> But that's a bit ugly.
>
> The driver could do that too for a quick hack, but it would
> be nice to query the correct value from the backend and use that.

This may be an opportunity to improve how PgJDBC finds out important
details about the backend in general.

Right now, lots of things require individual queries of GUCs via SHOW
commands or other misc queries. A typical JDBC session may do several
round trips before running its first real query, and it's only going to
get worse.

The JDBC driver really needs a way to grab everything it needs to know
efficiently during initial connection setup, with some extensibility so
connection parameters can specify other things to request and cache when
the connection is first established.

(Why extensibility? Because various layers - ORMs, query builders, etc
etc etc - tend to ask for more GUCs, and having the JDBC driver able to
immediately return them without more round trips would be great).

I'm dreaming of a situation where the JDBC driver sends a list of GUCs
it wants values for, caches the results, and most importantly keeps them
up to date by capturing the value sent in any SET commands issued via
the driver on that session.

This has been in the back of my mind a bit lately, as I look at the spew
of SHOW commands and similar shown when log_statement=all is enabled and
something like Hibernate/JPA2 is talking to the database.

--
Craig Ringer

Craig Ringer <ringerc@ringerc.id.au> writes:

> The JDBC driver really needs a way to grab everything it needs to know
> efficiently during initial connection setup, with some extensibility
> so connection parameters can specify other things to request and cache
> when the connection is first established.

Something like

  SELECT current_setting( 'shared_buffers' )
  UNION
  SELECT current_setting( 'data_directory' );

for some randomly chosen settings?  Of course the query will need to be
structured with key/values to be useful.


--
   Johann Oskarsson                http://www.2ndquadrant.com/    |[]
   PostgreSQL Development, 24x7 Support, Training and Services  --+--
                                                                  |
   Blog: http://my.opera.com/myrkraverk/blog/

On 06/08/2012 04:19 PM, Johann 'Myrkraverk' Oskarsson wrote:
> Craig Ringer<ringerc@ringerc.id.au>  writes:
>
>> The JDBC driver really needs a way to grab everything it needs to know
>> efficiently during initial connection setup, with some extensibility
>> so connection parameters can specify other things to request and cache
>> when the connection is first established.
>
> Something like
>
>    SELECT current_setting( 'shared_buffers' )
>    UNION
>    SELECT current_setting( 'data_directory' );
>
> for some randomly chosen settings?  Of course the query will need to be
> structured with key/values to be useful.

I was thinking more of running something like:

regress=#
SELECT name, setting
FROM pg_settings
WHERE name IN
('client_encoding','server_encoding','bytea_output','standard_conforming_strings',
'xmlbinary','DateStyle','TimeZone','array_nulls','backslash_quote','IntervalStyle',
'quote_all_identifiers', 'default_transaction_read_only');

             name               |    setting
-------------------------------+----------------
  array_nulls                   | on
  backslash_quote               | safe_encoding
  bytea_output                  | hex
  client_encoding               | UTF8
  default_transaction_read_only | off
  DateStyle                     | ISO, MDY
  IntervalStyle                 | postgres
  quote_all_identifiers         | off
  server_encoding               | UTF8
  standard_conforming_strings   | on
  TimeZone                      | Australia/West
  xmlbinary                     | base64
(11 rows)

... preferably combined with a mechanism for the server to notify (or
NOTIFY) the JDBC client when a pg_ctl reload or a SET causes settings to
change.

While the JDBC driver could try to track SET commands sent by its own
client, that would be trivially foiled by a SET wrapped in a function,
so really the server needs to tell the JDBC driver.

The main hurdles with using LISTEN/NOTIFY for that are:

- No triggers allowed on system tables so server code changes are
required; and

- If using LISTEN/NOTIFY, a way would be needed to make sure the
application never saw NOTIFY messages destined for PgJDBC (not hard with
payload notifies) and that a PgJDBC driver LISTEN name never clashed
with an application one.

- The JDBC driver won't find out about a settings change until the
end of a long-running query. Dunno of that matters, really.


The reason I'm interested in this is to allow the JDBC driver to do the
right thing for situations where server settings change how it should
interpret or send data. It'd also allow the JDBC driver to entirely cut
out server round trips and log spew on the backend when query
generators, ORMs, etc query settings. For example, one tool I use will
issue a `SHOW TRANSACTION ISOLATION;` painfully frequently; being able
to have PgJDBC answer that without a round trip by examining its cached
`transaction_isolation` would be great.


--
Craig Ringer

Craig Ringer <ringerc@ringerc.id.au> writes:
> On 06/08/2012 04:19 PM, Johann 'Myrkraverk' Oskarsson wrote:
>> Craig Ringer<ringerc@ringerc.id.au>  writes:
>>> The JDBC driver really needs a way to grab everything it needs to know
>>> efficiently during initial connection setup, with some extensibility
>>> so connection parameters can specify other things to request and cache
>>> when the connection is first established.

> I was thinking more of running something like:
> ... preferably combined with a mechanism for the server to notify (or
> NOTIFY) the JDBC client when a pg_ctl reload or a SET causes settings to
> change.

Running pg_settings() seems like a dead end to me, precisely because you
won't know about any subsequent changes; and most of the parameters that
JDBC might care about are changeable.  LISTEN/NOTIFY is not the answer
either, for the reasons you mention as well as some others.

There already is a mechanism to notify clients of changes in selected
GUC settings, but currently the set of parameters so reported is
hard-wired.  Possibly pgsql-hackers would consider a proposal to let
the GUC_REPORT flag get set on client-selected parameters.

            regards, tom lane

On Fri, Jun 8, 2012 at 6:47 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> There already is a mechanism to notify clients of changes in selected
> GUC settings, but currently the set of parameters so reported is
> hard-wired.  Possibly pgsql-hackers would consider a proposal to let
> the GUC_REPORT flag get set on client-selected parameters.

Somewhat off-topic, but along the same lines, it would also be worth
considering some mechanism to give the driver visibility into catalog
changes w.r.t. UDTs. I.e., right now, the only way for the driver to
sanely support custom types is to query the catalog for the oid of a
specific type name. However, if that type is subsequently dropped and
re-created and gets a different oid, the driver has no idea.
Unfortunately, I don't think there's a great way to support that
without protocol changes (whereas for the reporting flag, that's not
an issue).

On 06/09/2012 12:52 AM, Maciek Sakrejda wrote:
> On Fri, Jun 8, 2012 at 6:47 AM, Tom Lane<tgl@sss.pgh.pa.us>  wrote:
>> There already is a mechanism to notify clients of changes in selected
>> GUC settings, but currently the set of parameters so reported is
>> hard-wired.  Possibly pgsql-hackers would consider a proposal to let
>> the GUC_REPORT flag get set on client-selected parameters.
> Somewhat off-topic, but along the same lines, it would also be worth
> considering some mechanism to give the driver visibility into catalog
> changes w.r.t. UDTs. I.e., right now, the only way for the driver to
> sanely support custom types is to query the catalog for the oid of a
> specific type name. However, if that type is subsequently dropped and
> re-created and gets a different oid, the driver has no idea.
> Unfortunately, I don't think there's a great way to support that
> without protocol changes (whereas for the reporting flag, that's not
> an issue).

Is there currently a page with a list of changes to be included in the
next protocol revision? I could've sworn there was one around, but cant'
seem to find it. If there isn't, I'll pop one up on the wiki and link to
it from the todo.

Should put capability flags on the list too. So many difficult decisions
about defaults (like the bytea format issue) would be simplified by
having clients send a capability flag set. "Yup, I know about 'hex'
format bytea."

--
Craig Ringer

On 06/07/2012 06:50 AM, Craig Ringer wrote:
>
> This may be an opportunity to improve how PgJDBC finds out important
> details about the backend in general.
>
> Right now, lots of things require individual queries of GUCs via SHOW
> commands or other misc queries. A typical JDBC session may do several
> round trips before running its first real query, and it's only going to
> get worse.
> ...
> (Why extensibility? Because various layers - ORMs, query builders, etc
> etc etc - tend to ask for more GUCs, and having the JDBC driver able to
> immediately return them without more round trips would be great).

I certainly like that idea, but I hope someone won't mind doing the
quick, current-practice fix for setReadOnly() meanwhile.  There may
be a large and growing set of new layers that all want more GUCs,
but by contrast the set

  { x | x is defined by jdbc2 and doesn't currently work }

is smaller and bounded, and getReadOnly/setReadOnly are in it.

I'd submit a patch myself but this is really my first time looking
at the code, I haven't set up a development workspace, and I'm
hoping there's someone who could commit in 5 minutes what I'd be
tinkering with much of the day while I learn.

-Chap

> On 06/07/2012 04:11 AM, J Chapman Flack wrote:
>>
>> Hi,
>>
>> I've found this in the git head so it applies to current versions.
>>
>> AbstractJdbc2Connection initializes its readOnly member to a
>> hardcoded false instead of to 'show default_transaction_read_only'
>> from the backend.
>>
>> That means two things: 1. getReadOnly() is wrong in case the
>> backend has default_transaction_read_only on, and
>>
>> 2. setReadOnly(false) doesn't work in that case, because the value
>> is compared to readOnly and looks the same, so no set session
>> characteristics command is sent to the backend.
>>
>>
>> An application can work around (2) by always calling
>> setReadOnly(true);setReadOnly(false); if it wants to write.
>> But that's a bit ugly.
>>
>> The driver could do that too for a quick hack, but it would
>> be nice to query the correct value from the backend and use that.