Обсуждение: detecting serials in 8.1

8.1 has changed the default text for serials from something like
nextval('public.tab_col_seq'::text) to nextval('tab_col_seq'::regclass)

Kris Jurka
Index: src/schema/pgColumn.cpp
===================================================================
*** src/schema/pgColumn.cpp    (revision 5006)
--- src/schema/pgColumn.cpp    (working copy)
***************
*** 126,136 ****
  {
      wxString sql = GetQuotedTypename();

      if ((sql == wxT("int4") || sql == wxT("int8") ||
           sql == wxT("pg_catalog.int4") || sql == wxT("pg_catalog.int8"))
!         && GetDefault() == wxT("nextval('")
!                         + schema->GetName() + wxT(".") + GetTableName()
!                         + wxT("_") + GetName() + wxT("_seq'::text)"))
      {
          if (sql.Right(4) == wxT("int8"))
              sql = wxT("bigserial");
--- 126,150 ----
  {
      wxString sql = GetQuotedTypename();

+     // Technically this serial check can still fail for sequences that
+     // get created with non-default names.  Consider:
+     // CREATE SEQUENCE st_a_seq;
+     // CREATE TABLE st (a serial);
+     // Now the default's sequence is actually st_a_seq1.
+
+     wxString sequenceDefault;
+     if (GetDatabase()->BackendMinimumVersion(8, 1)) {
+         sequenceDefault = wxT("nextval('") + GetTableName()
+                         + wxT("_") + GetName() + wxT("_seq'::regclass)");
+     } else {
+         sequenceDefault = wxT("nextval('")
+                         + schema->GetName() + wxT(".") + GetTableName()
+                         + wxT("_") + GetName() + wxT("_seq'::text)");
+     }
+
      if ((sql == wxT("int4") || sql == wxT("int8") ||
           sql == wxT("pg_catalog.int4") || sql == wxT("pg_catalog.int8"))
!         && GetDefault() == sequenceDefault)
      {
          if (sql.Right(4) == wxT("int8"))
              sql = wxT("bigserial");

Kris Jurka wrote:
>
> 8.1 has changed the default text for serials from something like
> nextval('public.tab_col_seq'::text) to nextval('tab_col_seq'::regclass)

Applied with editing (didn't check for schema).

> +     // Technically this serial check can still fail for sequences that
> +     // get created with non-default names.  Consider:
> +     // CREATE SEQUENCE st_a_seq;
> +     // CREATE TABLE st (a serial);
> +     // Now the default's sequence is actually st_a_seq1.

This can't be created consistently using the CREATE TABLE foo (bar
serial) syntax, instead the column default syntax would need to be used.
I've put this on the TODO list, we'd need some discussion what reverse
engineering we really should show in such cases. This also has to
correspond with our column dialog, which will add pg_depend
automatically if adding a serial to an existing table (thus mimicking
the CREATE TABLE ... SERIAL stuff).

Regards,
Andreas

Andreas Pflug wrote:
> Kris Jurka wrote:
>> 8.1 has changed the default text for serials from something like
>> nextval('public.tab_col_seq'::text) to nextval('tab_col_seq'::regclass)
>
>
> Applied with editing (didn't check for schema).

Actually it turns out that whether the schema gets in there or not
depends on the search path when the table is created.  Consider the
following code:


CREATE schema s1;
CREATE schema s2;

SET search_path TO 's1';

CREATE TABLE t1 (a serial);
CREATE TABLE s2.t2 (a serial);

SET search_path TO 'public';

SELECT relname,attname,adsrc
FROM pg_class c, pg_attribute a, pg_attrdef d
WHERE c.oid = d.adrelid AND c.oid = a.attrelid
    AND a.attnum = d.adnum
    AND c.relname IN ('t1','t2');

Kris Jurka wrote:
> Andreas Pflug wrote:
>
>> Kris Jurka wrote:
>>
>>> 8.1 has changed the default text for serials from something like
>>> nextval('public.tab_col_seq'::text) to nextval('tab_col_seq'::regclass)
>>
>>
>>
>> Applied with editing (didn't check for schema).
>
>
> Actually it turns out that whether the schema gets in there or not
> depends on the search path when the table is created.

That's what pgSchema::GetPrefix does too.

Regards,
Andreas

Andreas Pflug wrote:
> Kris Jurka wrote:
>>
>> Actually it turns out that whether the schema gets in there or not
>> depends on the search path when the table is created.
>
>
> That's what pgSchema::GetPrefix does too.
>

That's fine as long as you assume that the search path never changes and
as long as you only have one schema in your search path.  I don't think
these are assumptions we can make.

Kris Jurka

Kris Jurka wrote:
> Andreas Pflug wrote:
>
>> Kris Jurka wrote:
>>
>>>
>>> Actually it turns out that whether the schema gets in there or not
>>> depends on the search path when the table is created.
>>
>>
>>
>> That's what pgSchema::GetPrefix does too.
>>
>
> That's fine as long as you assume that the search path never changes and
> as long as you only have one schema in your search path.  I don't think
> these are assumptions we can make.

It's _always_ fine, because GetSchema obeys the search path and the
reengineered SQL is meant to be used in a search path situation as it
was at the time of reengineering. There are plenty of other situations
where the reduced form (omitting search-pathed schema) won't work.

Regards,
Andreas

Andreas Pflug wrote:

> It's _always_ fine, because GetSchema obeys the search path and the
> reengineered SQL is meant to be used in a search path situation as it
> was at the time of reengineering. There are plenty of other situations
> where the reduced form (omitting search-pathed schema) won't work.
>

Let me back up and make clear what I'm saying.  The code I originally
submitted is not correct.  The code you committed is not correct either.
  To correctly determine what the default value for a serial will look
like, you need to know what the search_path was at table creation time.
  Knowing its current value is not relevent.

Run the following in psql.

SET search_path TO public;
CREATE schema s1;
CREATE TABLE s1.t1(a serial);
SET search_path TO s1;
CREATE TABLE s1.t2(a serial);
\d t1
\d t2

Note how one default includes the schema and the other doesn't.  Explain
how pgadmin can correctly determine the default value for both of these
tables.

Kris Jurka

Kris Jurka wrote:
> Andreas Pflug wrote:
>
>> It's _always_ fine, because GetSchema obeys the search path and the
>> reengineered SQL is meant to be used in a search path situation as it
>> was at the time of reengineering. There are plenty of other
>> situations where the reduced form (omitting search-pathed schema)
>> won't work.
>>
>
> Let me back up and make clear what I'm saying.  The code I originally
> submitted is not correct.  The code you committed is not correct
> either.  To correctly determine what the default value for a serial
> will look like, you need to know what the search_path was at table
> creation time.  Knowing its current value is not relevent.
>
> Run the following in psql.
>
> SET search_path TO public;
> CREATE schema s1;
> CREATE TABLE s1.t1(a serial);
> SET search_path TO s1;
> CREATE TABLE s1.t2(a serial);
> \d t1
> \d t2
>
> Note how one default includes the schema and the other doesn't.
> Explain how pgadmin can correctly determine the default value for both
> of these tables.
The problem of non-standard serials using pg_depend is on the TODO-list.
AFAICS the current implementation is better than before, and I don't
think adding more brains to this inferiour pattern matching approach
wouldn't make things better.

Regards,
Andreas