Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom methods for HSTORE and ANY ( method ) in psql #2

Closed
wants to merge 1 commit into from

Conversation

dinesh-it
Copy link

Please review and incorporate HSTORE and ANY methods used for psql.

@ribasushi
Copy link
Contributor

Hi, and sorry for the late reply!

First of all thank you very much for your contribution. As you noticed the original thread where you saw the implementation is quite old, and the feature has not yet been included in SQL::Abstract proper. This is due to architectural considerations: the 1.x series SQL::Abstract has no clean way of supporting dialects, and as such we are trying to only include "lowest common denominator" features in the main package. The pg-specific dialects fall outside of this category.

If you are interested in using this functionality with DBIx::Class specifically - let me know and I can tell you more about what is necessary to get this piece of code into the appropriate library (some things off the top of my head - there will definitely be need for tests, and handling of literals/-ident needs to be clarified).

Cheers!

@ribasushi ribasushi closed this May 22, 2014
@dinesh-it
Copy link
Author

Thanks for your reply, and yes I am trying to use it with DBIx::Class. Can you please tell me the appropriate library where I can add this code.

Thank you.

@timbunce
Copy link

Ping @ribasushi. I've a co-worker interested in this topic.

@ribasushi
Copy link
Contributor

Hi @dinesh-it, @timbunce,

Sorry for dropping the ball on this, github did not notify me (yet again) of activity post-close. This PR was dealing with two things - ANY and HSTORE, which I will address separately.

For ANY there is no need to add anything - it is covered by the current "generic operator" support:

rabbit@Ahasver:~$ perl -MSQL::Abstract -MDevel::Dwarn -e 'Dwarn [ SQL::Abstract->new->where({ Group => { -any => { -ident => "Prepay" } } }) ]'
[
  " WHERE ( Group ANY Prepay )"
]
rabbit@Ahasver:~$ perl -MSQL::Abstract -MDevel::Dwarn -e 'Dwarn [ SQL::Abstract->new->where({ Group => { -any => "Prepay" } }) ]'
[
  " WHERE ( Group ANY ? )",
  "Prepay"
]

For HSTORE implementing -> alone and giving it an arbitrary name of inhstore seems a bit shortsighted to me from an API-design standpoint. I think any future work needs to strive to cover the entire gamut of currently supported operators with an option open to future expansion.

Furthermore given how this is at this point firmly Postgres-only syntax (following their decision to repurpose ?), its best implementation venue would be a subclass of DBIx::Class::SQLMaker settable post-connect, or something like that.

These are just superficial thoughts off-the-cuff. I have never used HSTORE personally, hence do not have much to add in terms of design advice.

@timbunce
Copy link

Thanks @ribasushi. All makes sense.
(Minor aside: I suspect JSON/JSONB type usage, and thus operators, will become much more common than HSTORE over the next few years.)

@ilmari
Copy link
Member

ilmari commented Jan 19, 2015

@ribasushi: The generic operator support is not quite enough for ANY, since the syntax is foo = ANY(?) (see http://www.postgresql.org/docs/current/static/functions-comparisons.html#AEN19686):

$ reply
0> my $s = SQL::Abstract->new
[…]
1> $s->where({foo => { -any => { -value => [2] } }})
$res[1] = [
  ' WHERE ( foo ANY ? )',
  [
    2
  ]
]

2> $s->where({foo => { '=' => { -any => { -value => [2] }}}})
$res[2] = [
  ' WHERE ( foo = (ANY ?) )',
  [
    2
  ]
]

@ribasushi
Copy link
Contributor

@ilmari Ah Pg is bracket-sensitive isn't it. Then the comment about HSTORE applies here too - I believe it is of utmost importance to keep SQL::Abstract well... abstract. And keep any additional non-standard (standard in the practical sense of the word) behaviors to subclasses.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants