Sequel fails to detect correct pk-sequence #538

Closed
stmichael opened this Issue Aug 22, 2012 · 3 comments

Comments

Projects
None yet
2 participants
@stmichael

Hey

I came across a an issue when I tried to reset the pk-sequence of a table in postgres 9.0.4. The issue occurred with the following setup: I created a table, let's use 'old_addresses' for this example. The a renamed the table to 'new_addresses' with the rails API which renames the pk-sequence along with the table. So I have 'new_addresses' and 'new_addresses_id_seq'.

Let's look at what happens if I try to get the name of the pk-sequence of 'new_adresses'. In the file lib/sequel/adapters/shared/postgres.rb you have the following method to determine the pk-sequence name:

      def primary_key_sequence(table, opts={})
        quoted_table = quote_schema_table(table)
        @primary_key_sequences.fetch(quoted_table) do
          schema, table = schema_and_table(table)
          table = literal(table)
          sql = "#{SELECT_SERIAL_SEQUENCE_SQL} AND seq.relname = #{table}"
          sql << " AND name.nspname = #{literal(schema)}" if schema
          unless pks = fetch(sql).single_record
            sql = "#{SELECT_CUSTOM_SEQUENCE_SQL} AND t.relname = #{table}"
            sql << " AND name.nspname = #{literal(schema)}" if schema
            pks = fetch(sql).single_record
          end

          @primary_key_sequences[quoted_table] = if pks
            literal(SQL::QualifiedIdentifier.new(pks[:schema], LiteralString.new(pks[:sequence])))
          end
        end
      end

In the first SQL statement you try to get the sequence the "correct" way. SELECT_SERIAL_SEQUENCE_SQL is the statement to get all pk-sequences which you then filter by the table name. I took a closer look at the result of this statement. The relname of the record I'm looking for is 'new_addresses_id_seq'. But you add a condition that relname should be equal to the table name. This obviously doesn't work since 'new_addresses_id_seq' is not equal to 'new_addresses'.

I'm not sure if the statement is correct that way. Fact is that even without table renaming the first statement ALWAYS returns an empty result set. So the fallback with your second statement will be executed. In most cases that worked fine except with my renaming case.

I don't understand the statement in its entirety but I guess that you construct the name of the pk-sequence using the name of the primary key constraint. Unfortunately when I rename the table, the primary key constraint stays untouched (that is 'old_addresses_pkey'). So the fallback generates 'old_addresses_id_seq' which is wrong and causes my application to crash.

@jeremyevans

This comment has been minimized.

Show comment
Hide comment
@jeremyevans

jeremyevans Aug 22, 2012

Owner

First off, when reporting something like this, you should include a full backtrace and SQL log, and if possible a reproducible test case. Even if the code is wrong, I'm surprised you hit the error, since almost nothing calls that method. I assume you are either calling it directly or calling reset_primary_key_sequence (why?).

You mentioned you called a rails API that changed the table and sequence name. Are you sure that code handles things correctly?

It does look like the serial query is incorrect.

There are tests for this code, but no tests that include renaming the underlying table/sequence. I'll look into this and see if I can reproduce your issue and fix it.

Owner

jeremyevans commented Aug 22, 2012

First off, when reporting something like this, you should include a full backtrace and SQL log, and if possible a reproducible test case. Even if the code is wrong, I'm surprised you hit the error, since almost nothing calls that method. I assume you are either calling it directly or calling reset_primary_key_sequence (why?).

You mentioned you called a rails API that changed the table and sequence name. Are you sure that code handles things correctly?

It does look like the serial query is incorrect.

There are tests for this code, but no tests that include renaming the underlying table/sequence. I'll look into this and see if I can reproduce your issue and fix it.

@jeremyevans

This comment has been minimized.

Show comment
Hide comment
@jeremyevans

jeremyevans Aug 22, 2012

Owner

I've fixed the serial sequence query. Can you try the master branch and see if that fixes your issue?

Owner

jeremyevans commented Aug 22, 2012

I've fixed the serial sequence query. Can you try the master branch and see if that fixes your issue?

@stmichael

This comment has been minimized.

Show comment
Hide comment
@stmichael

stmichael Aug 23, 2012

That fixed the issue. Thank you very much.

That fixed the issue. Thank you very much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment