pass table and column to custom sql function

[twelve17]

In trying to figure out some NULL sorting handling issues, I ran into #10, and in particular the suggestion to use COALESCE to turn a null value to a non-value.

In order to allow this kind of functionality for dynamic queries, I am suggesting that the table and column name obtained from the connection be passed to the sql function. In other words, in here, a change along the lines of:

          table = connection.quote_table_name(scope.table_name)
          column = connection.quote_column_name(column.name)

          if sql
            sql.respond_to?(:call) ? sql.call(table, column) : sql
          else
            "#{table}."\
            "#{column}"

This allows me to call COALESCE using the resolved table name, as I don't have access to the connection in the custom sql proc.

If you're open to this, I can submit a PR.

Thank you!

[glebm]

Null ordering has been implemented and released in v0.4.0. Did you run into issues with using the new option?

[twelve17]

@glebm I am. I'm using v0.4.0. Seems like it doesn't think there are more records, even though I know there are. I did a test by COALESCEing the column's NULL values to empty strings, and it seems to work.

Also, it could be because I'm using dynamic queries. I'm not sure.

[glebm]

What does the SQL look like with e.g. nulls: :last and no COALESCE? You can find it in the logs or by calling to_sql on the final relation.

[glebm]

By the way, I don't mind allowing a proc as sql: but I want to also figure out why it doesn't work for you with just nulls: :last.

[twelve17]

Interestingly, I had nulls set to :first, maybe that was my problem? If I set nulls to :last, it seems to work, but then again I'm not reaching the end of the data set.

Here is the SQL for both :first and :last. title is the column that is mostly made up of NULL values.

seek_args: [[:title, :desc, {:nulls=|:first}]]
Entry Load (1.5ms)  SELECT  `entries`.* FROM `entries` ORDER BY `entries`.`title` IS NULL DESC, `entries`.`title` DESC, `entries`.`id` ASC LIMIT 20
Entry Load (0.8ms)  SELECT  `entries`.* FROM `entries` WHERE `entries`.`id` = 1 LIMIT 1
Entry Load (1.0ms)  SELECT  `entries`.* FROM `entries` WHERE (`entries`.`title` IS NULL AND (`entries`.`title` IS NULL OR `entries`.`title` IS NULL AND `entries`.`id` | 1)) ORDER BY `entries`.`title` IS NULL ASC, `entries`.`title` ASC, `entries`.`id`     DESC LIMIT 1



seek_args: [[:title, :desc, {:nulls=|:last}]]
Entry Load (1.0ms)  SELECT  `entries`.* FROM `entries` ORDER BY `entries`.`title` DESC, `entries`.`id` ASC LIMIT 20
Entry Load (0.7ms)  SELECT  `entries`.* FROM `entries` WHERE `entries`.`id` = 1 LIMIT 1
Entry Load (0.8ms)  SELECT  `entries`.* FROM `entries` WHERE (`entries`.`title` IS NOT NULL AND (`entries`.`title` IS NOT NULL OR `entries`.`title` IS NULL AND `entries`.`id` | 1)) ORDER BY `entries`.`title` ASC, `entries`.`id` DESC LIMIT 1

Let me know if I can help further.

[glebm]

What database are you using?

[twelve17]

mysqld 5.7.19

[glebm]

I think I see a bug with that query. Just to confirm, what happens if you set OrderQuery.wrap_top_level_or = false?

[twelve17]

search#records_for: seek_args: [[:title, :desc, {:nulls=|:first}]]
Entry Load (1.0ms)  SELECT  `entries`.* FROM `entries` ORDER BY `entries`.`title` IS NULL DESC, `entries`.`title` DESC, `entries`.`id` ASC LIMIT 20
Entry Load (0.9ms)  SELECT  `entries`.* FROM `entries` WHERE `entries`.`id` = 1 LIMIT 1
Entry Load (2.0ms)  SELECT  `entries`.* FROM `entries` WHERE (`entries`.`title` IS NULL OR `entries`.`title` IS NULL AND `entries`.`id` | 1) ORDER BY `entries`.`title` IS NULL ASC, `entries`.`title`   ASC, `entries`.`id` DESC LIMIT 1

(This seems to work, by the way.)

[glebm]

Thanks, I've opened a separate issue about this bug

[twelve17]

(On a side note, I was not asking to add proc support to the sql param-- it already seems to exist. The suggestion was just to add passing in the table and column values as params to the call.)