Add #qualified method / allow compact join tables qualification #27

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
2 participants
Contributor

funny-falcon commented Mar 21, 2011

My main wish is to use simpler joins block.
This patch allows using joins like this (simplified):
DB[:clients].join(:period){|p, cl, js|
{p[:client_id] => cl[:id]} & (p[:time_from] >= some_date)
}
instead of
DB[:clients].join(:period){|p, cl, js|
{:client_id.qualify(p) => :id.qualify(cl)} & (:time_from.qualify(p) >= some_date)
}

To accomplish this, the patch adds method #qualified to Symbol, Identifier and QualifiedIdentifier
and #[] as shortcut to #qualified to Identifier and QualifiedIdentifier,
this methods perform as column.qualify(table) == table.qualified(column)

It adds method #identifier to Identifier for backward compatibility.

allow compact join tables qualification
It allows using joins like this (simplified):
    DB[:clients].join(:period){|p, cl, js|
        {p[:client_id] => cl[:id]} & (p[:time_from] >= some_date)
    }
instead of
    DB[:clients].join(:period){|p, cl, js|
        {:client_id.qualify(p) => :id.qualify(cl)} & (:time_from.qualify(p) >= some_date)
    }

It adds method #qualified to Symbol, Identifier and QualifiedIdentifier
 and #[] as shortcut to #qualified to Identifier and QualifiedIdentifier
which performs as `column.qualify(table) == table.qualified(column)`

It adds method #identifier to Identifier for backward compatibility
Owner

jeremyevans commented Mar 21, 2011

I'm not sure I like the use of #[] on Identifier and QualifiedIdentifier to mean qualification. I was thinking of using it to create SQL functions in the future, similar to how Symbol#[] creates SQL functions.

I'm not opposed to adding #qualified, or an #indentifier method that returns self.

In regards to your specific patch, you aren't allowed to use methods like #identifier inside of the Sequel codebase, as it breaks Sequel when someone uses the SEQUEL_NO_CORE_EXTENSIONS constant or environment variable. You have to create identifiers the long way. And I don't think it's a good idea to automatically convert things to identifiers when passing them to the join_table block, as it could break existing code expecting a Symbol.

Can you post to the Sequel Google Group to get some feedback from the Sequel community?

Contributor

funny-falcon commented Mar 21, 2011

similar to how Symbol#[] creates SQL functions

oh, I've missed this. Guides point to Symbol#sql_function method and VirtualRow usage (here and here) and Symbol#[] is mentioned only in rdoc
And Symbol#[] is not redefined in 1.9 I use for development.

My first thought were about Identifier#method_missing ala VirtualRow. But Identifier itself has a lot of methods, so that it is not save, I think.
Second try were Identifier#c method. p.c(:client_id) looks clear but not as clear as p[:client_id], and I thought #[] is not abused.

And I don't think it's a good idea to automatically convert things to identifiers when passing them to the join_table block, as it could break existing code expecting a Symbol.

Hmm. Is there any practical usage of it except in :symbol.qualify? I added #identifier so that it could mimic Symbol in Sequel meaning.

you aren't allowed to use methods like #identifier

I'll fix it now.

Can you post to the Sequel Google Group to get some feedback from the Sequel community?

Ok.

Owner

jeremyevans commented Mar 21, 2011

I don't like to use method_missing except when it is necessary (like for VirtualRow). I think #c is not a good method name as it's not possible to guess at what it does by looking at it.

The issue with converting Symbols to Identifiers is that they are not the same. User code could do things like s.to_s or some_hash[s] that will fail if you automatically convert.

Contributor

funny-falcon commented Mar 21, 2011

I don't like to use method_missing

me too

I think #c is not a good method name as it's not possible to guess at what it does by looking at it.

Oh... I wanted it to be short... #qualified is long... May be #field or #column?

Owner

jeremyevans commented Mar 21, 2011

qualified is 3-4 characters longer but it is also more accurate and descriptive. For example, if you are using the symbol as a schema to do something like :schema.qualified(:table), it wouldn't make sense to do :schema.column(:table).

Contributor

funny-falcon commented Mar 22, 2011

but #column (or #field) could be an alias for #qualified, so that one could write :schema.qualified(:table) and :table.column(:column)

Contributor

funny-falcon commented Mar 22, 2011

Considering Identifier#[]: may be use other method for calling sql functions? I think it is less common task than qualifying.
Shortest would be #func.
And it opens way to simpler calling qualified functions
For example, I need to call select voicet_back.client_sum_ue(1024, 'time_from', 'time_to') as sum_ue.
With new syntax I could write DB[].select{ voicet_back[:client_sum_ue].func(1024, time_from, time_to).as(:sum_ue) } .
Currently I ought to use placeholders: DB[].select('voicet_back.client_sum_ue(?, ?, ?)'.lit(1024, time_from, time_to).as(:sum_ue)) . I don't like using placeholders in Sequel. It is not Sequel way, I think.

Owner

jeremyevans commented Mar 22, 2011

I'm sure that the extensions you are proposing make sense in your app. However, that does not necessarily imply that they make sense to be included in Sequel itself.

So far on the Google Group, nobody has responded to your post regarding this. We'll leave the post out there for a while, but unless other Sequel users think this is a good idea, I don't plan on merging it.

Now, one of the beauties of ruby is that you can implement such code yourself. So your app can have these changes without affecting anyone else's app. This is a good thing. You don't need to get all of your Sequel extensions included in Sequel to be able to use them in your app.

As for your schema qualified function, you can currently do:

:"voicet_back.client_sum_ue".sql_function(1024, time_from, time_to)

In the future, it's possible that the following will work:

:client_sum_ue.qualify(:voicet_back).sql_function(1024, time_from, time_to)

Currently sql_function is not implemented for Identifiers and QualifiedIdentifiers, but it certainly could be.

Contributor

funny-falcon commented Mar 22, 2011

So far on the Google Group, nobody has responded to your post regarding this.

Yeah, I see.
Well, I didn't expect it would be merged :) Just a try.
I like you have a vision of library!

Owner

jeremyevans commented Mar 30, 2011

Looks like nobody on the Google Group said they wanted this, so I'm closing the pull request

Contributor

funny-falcon commented Mar 30, 2011

Ok, no problem. I will use monkey pathing.

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