Sequel::SQL::QualifiedIdentifier is stringified incorrectly when creating a temporary table

[whoward]

Complete Description of Issue

When we attempt to create a temporary table with the name specified as a Sequel::SQL::QualifiedIdentifier the table name is not qualified when the SQL statement is generated. This IRB session should illustrate the error:

irb(main):001> Sequel::VERSION
=> "5.82.0"
irb(main):002> RUBY_DESCRIPTION
=> "ruby 3.3.3 (2024-06-12 revision f1c7b6f435) [arm64-darwin23]"
irb(main):003> DB = Sequel.connect(ENV.fetch('DATABASE_URL'), logger: Logger.new($stderr)); nil
I, [2024-07-10T23:21:29.740483 #38919]  INFO -- : (0.000188s) SET standard_conforming_strings = ON
I, [2024-07-10T23:21:29.740677 #38919]  INFO -- : (0.000118s) SET client_min_messages = 'WARNING'
I, [2024-07-10T23:21:29.740802 #38919]  INFO -- : (0.000107s) SET DateStyle = 'ISO'
=> nil
irb(main):004> name = Sequel.qualify(:public, :some_name)
=> #<Sequel::SQL::QualifiedIdentifier @table=>:public, @column=>:some_name>
irb(main):005> DB.create_table(name, temp: true) { column :some_column, :text }
I, [2024-07-10T23:22:00.756191 #38919]  INFO -- : (0.010112s) CREATE TEMPORARY TABLE "#<Sequel::SQL::QualifiedIdentifier:0x000000011e2f3838>" ("some_column" text)

Please note this example is illustrative, in practice I am creating the table in a schema other than public.

This behaviour does not occur when creating the table without the temp: true option.

Simplest Possible Self-Contained Example Showing the Bug

DB = Sequel.connect(ENV.fetch('DATABASE_URL'), logger: Logger.new($stderr))
name = Sequel.qualify(:public, :some_name)
DB.create_table(name, temp: true) { column :some_column, :text }

Full Backtrace of Exception (if any)

N/A

SQL Log (if any)

I, [2024-07-10T23:21:29.740483 #38919] INFO -- : (0.000188s) SET standard_conforming_strings = ON I, [2024-07-10T23:21:29.740677 #38919] INFO -- : (0.000118s) SET client_min_messages = 'WARNING' I, [2024-07-10T23:21:29.740802 #38919] INFO -- : (0.000107s) SET DateStyle = 'ISO' I, [2024-07-10T23:22:00.756191 #38919] INFO -- : (0.010112s) CREATE TEMPORARY TABLE "#" ("some_column" text)

Ruby Version

ruby 3.3.3 (2024-06-12 revision f1c7b6f435) [arm64-darwin23]

Sequel Version

5.82.0

[jeremyevans]

Thank you very much for the report. I'll work on fixing this right away.

[jeremyevans]

After some research, this is deliberate, introduced in 1fd397626413b394bf771ea69669efc7b91c92f7 , because PostgreSQL doesn't actually support what you want (https://www.postgresql.org/docs/current/sql-createtable.html, "Temporary tables exist in a special schema, so a schema name cannot be given when creating a temporary table"), and using Sequel's default quoting method caused an issue if the default_schema was set.

You are using PostgreSQL, and your code wouldn't work even if Sequel produced SQL that included a schema qualified table. Are you sure you want Sequel to generate SQL that PostgreSQL would reject?

[jeremyevans]

I think I'll fix this issue even if it results in SQL that PostgreSQL wouldn't support. If an SQL::QualifiedIdentifier is passed, then Sequel should use a schema qualified table, because that is the explicit intention of the user.

[whoward]

Ah you are correct, I was intending to use it incorrectly but didn't realize this was not supported. I think your decision however to stringify the qualified identifier makes sense as it would be less surprising to the user and allow them to arrive at the real problem sooner by seeing a useful error message from Postgres.

[jeremyevans]

This ended up being significantly more complicated than I expected, as it touches 4 database adapters in addition to the default support. Here's a diff, but I'll need to do more testing with it before I commit it:

diff --git a/lib/sequel/adapters/jdbc/derby.rb b/lib/sequel/adapters/jdbc/derby.rb
index b97edb7c3..dffdd2080 100644
--- a/lib/sequel/adapters/jdbc/derby.rb
+++ b/lib/sequel/adapters/jdbc/derby.rb
@@ -115,7 +115,7 @@ module Sequel
         # Temporary table creation on Derby uses DECLARE instead of CREATE.
         def create_table_prefix_sql(name, options)
           if options[:temp]
-            "DECLARE GLOBAL TEMPORARY TABLE #{quote_identifier(name)}"
+            "DECLARE GLOBAL TEMPORARY TABLE #{create_table_table_name_sql(name, options)}"
           else
             super
           end
diff --git a/lib/sequel/adapters/shared/db2.rb b/lib/sequel/adapters/shared/db2.rb
index 83cd6a442..235f853f2 100644
--- a/lib/sequel/adapters/shared/db2.rb
+++ b/lib/sequel/adapters/shared/db2.rb
@@ -198,7 +198,7 @@ module Sequel
       # http://www.ibm.com/developerworks/data/library/techarticle/dm-0912globaltemptable/
       def create_table_prefix_sql(name, options)
         if options[:temp]
-          "DECLARE GLOBAL TEMPORARY TABLE #{quote_identifier(name)}"
+          "DECLARE GLOBAL TEMPORARY TABLE #{create_table_table_name_sql(name, options)}"
         else
           super
         end
diff --git a/lib/sequel/adapters/shared/mssql.rb b/lib/sequel/adapters/shared/mssql.rb
index e459d3e9d..ce90abd28 100644
--- a/lib/sequel/adapters/shared/mssql.rb
+++ b/lib/sequel/adapters/shared/mssql.rb
@@ -379,9 +379,19 @@ module Sequel
       # a regular and temporary table, with temporary table names starting with
       # a #.
       def create_table_prefix_sql(name, options)
-        "CREATE TABLE #{quote_schema_table(options[:temp] ? "##{name}" : name)}"
+        "CREATE TABLE #{create_table_table_name_sql(name, options)}"
       end
-      
+
+      # The SQL to use for the table name for a temporary table.
+      def create_table_temp_table_name_sql(name, _options)
+        case name
+        when String, Symbol
+          "##{name}"
+        else
+          raise Error, "temporary table names must be strings or symbols on Microsoft SQL Server"
+        end
+      end
+
       # MSSQL doesn't support CREATE TABLE AS, it only supports SELECT INTO.
       # Emulating CREATE TABLE AS using SELECT INTO is only possible if a dataset
       # is given as the argument, it can't work with a string, so raise an
diff --git a/lib/sequel/adapters/shared/postgres.rb b/lib/sequel/adapters/shared/postgres.rb
index 4087ebba2..cda8e1a94 100644
--- a/lib/sequel/adapters/shared/postgres.rb
+++ b/lib/sequel/adapters/shared/postgres.rb
@@ -1429,7 +1429,7 @@ module Sequel
           'UNLOGGED '
         end

-        "CREATE #{prefix_sql}TABLE#{' IF NOT EXISTS' if options[:if_not_exists]} #{options[:temp] ? quote_identifier(name) : quote_schema_table(name)}"
+        "CREATE #{prefix_sql}TABLE#{' IF NOT EXISTS' if options[:if_not_exists]} #{create_table_table_name_sql(name, options)}"
       end

       # SQL for creating a table with PostgreSQL specific options
diff --git a/lib/sequel/database/schema_methods.rb b/lib/sequel/database/schema_methods.rb
index df77cc7d7..c7cdd0c9f 100644
--- a/lib/sequel/database/schema_methods.rb
+++ b/lib/sequel/database/schema_methods.rb
@@ -775,7 +775,21 @@ module Sequel

     # SQL fragment for initial part of CREATE TABLE statement
     def create_table_prefix_sql(name, options)
-      "CREATE #{temporary_table_sql if options[:temp]}TABLE#{' IF NOT EXISTS' if options[:if_not_exists]} #{options[:temp] && !name.is_a?(SQL::QualifiedIdentifier) ? quote_identifier(name) : quote_schema_table(name)}"
+      "CREATE #{temporary_table_sql if options[:temp]}TABLE#{' IF NOT EXISTS' if options[:if_not_exists]} #{create_table_table_name_sql(name, options)}"
+    end
+
+    # The SQL to use for a table name when creating a table.
+    # Use of the :temp option can result in different SQL,
+    # because the rules for temp table naming can differ
+    # between databases, and temp tables should not use the
+    # default_schema.
+    def create_table_table_name_sql(name, options)
+      options[:temp] ? create_table_temp_table_name_sql(name, options) : quote_schema_table(name)
+    end
+
+    # The SQL to use for the table name for a temporary table.
+    def create_table_temp_table_name_sql(name, _options)
+      name.is_a?(String) ? quote_identifier(name) : literal(name)
     end

     # SQL fragment for initial part of CREATE VIEW statement