Permalink
Browse files

Refactor and modularize Dataset#sql_select, and refactor database ada…

…pters that override it

This is a long overdue change to Sequel that makes select_sql more
modular and makes database adapters to only override the parts they
need to override instead of overriding the entire method (which
requires a lot of code duplication).

Unfortunately, I can't test the changes I made to most of the
adapters, as I don't have access to the underlying databases.  If you
use one of the following adapters, please test this commit and
make sure it works:

* Informix
* MSSQL
* Oracle

This also makes some changes to existing code.  Attempting to do a
SELECT DISTINCT ON will now raise an error with the MySQL or Oracle
adapters, as they support DISTINCT but not DISTINCT ON.  Previously,
the MySQL adapter would raise a DatabaseError when you ran the query,
and the Oracle adapter would silently change the DISTINCT ON to a
plain DISTINCT, which potentially changes the intended meaning.

The refactoring is done by splitting the public select_sql method
into a bunch of private select_*_sql methods.  The
select_clause_order private method is the list of methods to call
in order.  So if it was [:columns, :from], it would call
select_columns_sql and then select_from_sql to build the string.
All of the private select_*_sql methods take 2 arguments, the current
SQL string and the option hash to use.  The SQL string passed should
be modified in place.  Database adapters have been changed to
override just the select_*_sql methods they need, as well as
select_clause_order if they need the clauses in a different order
than the default.

Another small change made is that the Dataset#column_list private
method now treats a nil argument the same as an [] argument.  This
makes other internal code that uses the method nicer.
  • Loading branch information...
1 parent d407190 commit a0f0dec1b2ce7c4304f186d3a9d1796166480403 @jeremyevans committed Nov 19, 2008
View
@@ -1,5 +1,7 @@
=== HEAD
+* Make MySQL and Oracle adapters raise an Error if asked to do a SELECT DISTINCT ON (jeremyevans)
+
* Set standard_conforming_strings = ON by default when using PostgreSQL, turn off with Sequel::Postgres.force_standard_strings = false (jeremyevans) (#247)
* Fix Database#rename_table when using PostgreSQL (jeremyevans) (#248)
@@ -39,6 +39,8 @@ def execute(sql, opts={})
end
class Dataset < Sequel::Dataset
+ SELECT_CLAUSE_ORDER = %w'limit distinct columns from join where group having order union intersect except'.freeze
+
def literal(v)
case v
when Time
@@ -50,18 +52,6 @@ def literal(v)
end
end
- def select_sql(opts = nil)
- limit = opts.delete(:limit)
- offset = opts.delete(:offset)
- sql = super
- if limit
- limit = "FIRST #{limit}"
- offset = offset ? "SKIP #{offset}" : ""
- sql.sub!(/^select /i,"SELECT #{offset} #{limit} ")
- end
- sql
- end
-
def fetch_rows(sql, &block)
execute(sql) do |cursor|
begin
@@ -72,6 +62,17 @@ def fetch_rows(sql, &block)
end
self
end
+
+ private
+
+ def select_clause_order
+ SELECT_CLAUSE_ORDER
+ end
+
+ def select_limit_sql(sql, opts)
+ sql << " SKIP #{opts[:offset]}" if opts[:offset]
+ sql << " FIRST #{opts[:limit]}" if opts[:limit]
+ end
end
end
end
@@ -35,6 +35,8 @@ def rollback_transaction_sql
end
module DatasetMethods
+ SELECT_CLAUSE_ORDER = %w'intersect except limit distinct columns from with join where group order having union'.freeze
+
def complex_expression_sql(op, args)
case op
when :'||'
@@ -75,72 +77,31 @@ def quoted_identifier(name)
"[#{name}]"
end
- # Formats a SELECT statement using the given options and the dataset
- # options.
- def select_sql(opts = nil)
- opts = opts ? @opts.merge(opts) : @opts
-
- if sql = opts[:sql]
- return sql
- end
-
- # ADD TOP to SELECT string for LIMITS
- if limit = opts[:limit]
- top = "TOP #{limit} "
- raise Error, "Offset not supported" if opts[:offset]
- end
-
- columns = opts[:select]
- # We had to reference const WILDCARD with its full path, because
- # the Ruby constant scope rules played against us (it was resolving it
- # as Sequel::Dataset::DatasetMethods::WILDCARD).
- select_columns = columns ? column_list(columns) : Sequel::Dataset::WILDCARD
-
- if distinct = opts[:distinct]
- distinct_clause = distinct.empty? ? "DISTINCT" : "DISTINCT ON (#{expression_list(distinct)})"
- sql = "SELECT #{top}#{distinct_clause} #{select_columns}"
- else
- sql = "SELECT #{top}#{select_columns}"
- end
-
- if opts[:from]
- sql << " FROM #{source_list(opts[:from])}"
- end
-
- # ADD WITH to SELECT string for NOLOCK
- if with = opts[:with]
- sql << " WITH #{with}"
- end
-
- if join = opts[:join]
- join.each{|j| sql << literal(j)}
- end
+ private
- if where = opts[:where]
- sql << " WHERE #{literal(where)}"
- end
+ def select_clause_order
+ SELECT_CLAUSE_ORDER
+ end
- if group = opts[:group]
- sql << " GROUP BY #{expression_list(group)}"
- end
+ # EXCEPT is not supported by MSSQL
+ def select_except_sql(sql, opts)
+ raise(Error, "EXCEPT not supported") if opts[:except]
+ end
- if order = opts[:order]
- sql << " ORDER BY #{expression_list(order)}"
- end
+ # INTERSET is not supported by MSSQL
+ def select_intersect_sql(sql, opts)
+ raise(Error, "INTERSECT not supported") if opts[:intersect]
+ end
- if having = opts[:having]
- sql << " HAVING #{literal(having)}"
- end
+ # MSSQL uses TOP for limit, with no offset support
+ def select_limit_sql(sql, opts)
+ raise(Error, "OFFSET not supported") if opts[:offset]
+ sql << " TOP #{opts[:limit]}" if opts[:limit]
+ end
- if union = opts[:union]
- sql << (opts[:union_all] ? \
- " UNION ALL #{union.sql}" : " UNION #{union.sql}")
- end
-
- raise Error, "Intersect not supported" if opts[:intersect]
- raise Error, "Except not supported" if opts[:except]
-
- sql
+ # MSSQL uses the WITH statement to lock tables
+ def select_with_sql(sql, opts)
+ sql << " WITH #{opts[:with]}" if opts[:with]
end
end
end
@@ -253,6 +253,16 @@ def update_sql(values, opts = nil)
sql
end
+
+ private
+
+ # MySQL doesn't support DISTINCT ON
+ def select_distinct_sql(sql, opts)
+ if opts[:distinct]
+ raise(Error, "DISTINCT ON not supported by MySQL") unless opts[:distinct].empty?
+ sql << " DISTINCT"
+ end
+ end
end
end
end
@@ -13,69 +13,35 @@ def table_exists?(name)
end
module DatasetMethods
+ SELECT_CLAUSE_ORDER = %w'distinct columns from join where group having union intersect except order limit'.freeze
+
def empty?
db[:dual].where(exists).get(1) == nil
end
- # Formats a SELECT statement using the given options and the dataset
- # options.
- def select_sql(opts = nil)
- opts = opts ? @opts.merge(opts) : @opts
-
- if sql = opts[:sql]
- return sql
- end
-
- columns = opts[:select]
- select_columns = columns ? column_list(columns) : '*'
- sql = opts[:distinct] ? \
- "SELECT DISTINCT #{select_columns}" : \
- "SELECT #{select_columns}"
-
- if opts[:from]
- sql << " FROM #{source_list(opts[:from])}"
- end
-
- if join = opts[:join]
- join.each{|j| sql << literal(j)}
- end
-
- if where = opts[:where]
- sql << " WHERE #{literal(where)}"
- end
-
- if group = opts[:group]
- sql << " GROUP BY #{expression_list(group)}"
- end
-
- if having = opts[:having]
- sql << " HAVING #{literal(having)}"
- end
+ private
- if union = opts[:union]
- sql << (opts[:union_all] ? \
- " UNION ALL #{union.sql}" : " UNION #{union.sql}")
- elsif intersect = opts[:intersect]
- sql << (opts[:intersect_all] ? \
- " INTERSECT ALL #{intersect.sql}" : " INTERSECT #{intersect.sql}")
- elsif except = opts[:except]
- sql << (opts[:except_all] ? \
- " EXCEPT ALL #{except.sql}" : " EXCEPT #{except.sql}")
- end
+ def select_clause_order
+ SELECT_CLAUSE_ORDER
+ end
- if order = opts[:order]
- sql << " ORDER BY #{expression_list(order)}"
+ # Oracle doesn't support DISTINCT ON
+ def select_distinct_sql(sql, opts)
+ if opts[:distinct]
+ raise(Error, "DISTINCT ON not supported by Oracle") unless opts[:distinct].empty?
+ sql << " DISTINCT"
end
+ end
+ # Oracle requires a subselect to do limit and offset
+ def select_limit_sql(sql, opts)
if limit = opts[:limit]
if (offset = opts[:offset]) && (offset > 0)
- sql = "SELECT * FROM (SELECT raw_sql_.*, ROWNUM raw_rnum_ FROM(#{sql}) raw_sql_ WHERE ROWNUM <= #{limit + offset}) WHERE raw_rnum_ > #{offset}"
+ sql.replace("SELECT * FROM (SELECT raw_sql_.*, ROWNUM raw_rnum_ FROM(#{sql}) raw_sql_ WHERE ROWNUM <= #{limit + offset}) WHERE raw_rnum_ > #{offset}")
else
- sql = "SELECT * FROM (#{sql}) WHERE ROWNUM <= #{limit}"
+ sql.replace("SELECT * FROM (#{sql}) WHERE ROWNUM <= #{limit}")
end
end
-
- sql
end
end
end
@@ -399,6 +399,7 @@ module DatasetMethods
QUERY_PLAN = 'QUERY PLAN'.to_sym
ROW_EXCLUSIVE = 'ROW EXCLUSIVE'.freeze
ROW_SHARE = 'ROW SHARE'.freeze
+ SELECT_CLAUSE_ORDER = %w'distinct columns from join where group having order limit union intersect except lock'.freeze
SHARE = 'SHARE'.freeze
SHARE_ROW_EXCLUSIVE = 'SHARE ROW EXCLUSIVE'.freeze
SHARE_UPDATE_EXCLUSIVE = 'SHARE UPDATE EXCLUSIVE'.freeze
@@ -518,19 +519,6 @@ def multi_insert_sql(columns, values)
["INSERT INTO #{source_list(@opts[:from])} (#{identifier_list(columns)}) VALUES #{values}"]
end
- # Support lock mode, allowing FOR SHARE and FOR UPDATE queries.
- def select_sql(opts = nil)
- row_lock_mode = opts ? opts[:lock] : @opts[:lock]
- sql = super
- case row_lock_mode
- when :update
- sql << FOR_UPDATE
- when :share
- sql << FOR_SHARE
- end
- sql
- end
-
private
# Call execute_insert on the database object with the given values.
@@ -544,6 +532,21 @@ def insert_returning_pk_sql(*values)
insert_returning_sql(pk ? Sequel::SQL::Identifier.new(pk) : 'NULL'.lit, *values)
end
+ # The order of clauses in the SELECT SQL statement
+ def select_clause_order
+ SELECT_CLAUSE_ORDER
+ end
+
+ # Support lock mode, allowing FOR SHARE and FOR UPDATE queries.
+ def select_lock_sql(sql, opts)
+ case opts[:lock]
+ when :update
+ sql << FOR_UPDATE
+ when :share
+ sql << FOR_SHARE
+ end
+ end
+
# The version of the database server
def server_version
db.server_version(@opts[:server])
Oops, something went wrong. Retry.

0 comments on commit a0f0dec

Please sign in to comment.