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

Handle empty arrays in -in and -between in subclasses #22

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jjatria
Copy link

@jjatria jjatria commented Sep 7, 2022

Subclasses that override _where_field_IN and _where_field_BETWEEN and conditionally re-dispatch to the default handlers in SQL::Abstract were not correctly handling empty array references passed as arguments to -in, -between, and their negated counterparts.

This behaviour was broken in the refactor that lead to 2.000000, and more specifically in 1.90_03, going from

$ perl -Ilib -MSQL::Abstract -E '
  say "version " . $SQL::Abstract::VERSION;
  package X {
    use parent "SQL::Abstract";
    use mro "c3";
    sub _where_field_IN { shift->next::method(@_) }
  }
  say X->new->select( table => ["*"], { foo => { -in => [] } } )
'
version 1.9002
SELECT * FROM table WHERE 0=1

to

$ perl -Ilib -MSQL::Abstract -E '
  say "version " . $SQL::Abstract::VERSION;
  package X {
    use parent "SQL::Abstract";
    use mro "c3";
    sub _where_field_IN { shift->next::method(@_) }
  }
  say X->new->select( table => ["*"], { foo => { -in => [] } } )
'
version 1.9003
SELECT * FROM table WHERE foo IN ( )

and resulting in illegal SQL.

This was breaking behaviour that resulted in downstream issues affecting at least SQL::Abstract::More (see damil/SQL-Abstract-More#20).

Likewise, a class overriding _where_field_BETWEEN was not triggering the parameter validation that disallowed empty array references or undefined values to be used as parameters to -between and its negation:

$ perl -Ilib -MSQL::Abstract -E '
  say "version " . $SQL::Abstract::VERSION;
  package X {
    use parent "SQL::Abstract";
    use mro "c3";
    sub _where_field_BETWEEN { shift->next::method(@_) }
  }
  say X->new->select( table => ["*"], { foo => { -between => [] } } )
'
version 1.9003
SELECT * FROM table WHERE ( foo BETWEEN AND )

This change ensures that the path taken by subclasses that override those methods triggers equivalent code paths to those that don't.

Fixes https://rt.cpan.org/Ticket/Display.html?id=142341

Subclasses that override `_where_field_IN` and `_where_field_BETWEEN`
and conditionally re-dispatch to the default handlers in SQL::Abstract
were not correctly handling empty array references passed as arguments
to `-in`, `-between`, and their negated counterparts.

This behaviour was broken in the refactor that lead to 2.000000, and
more specifically in 1.90_03, going from

    $ perl -Ilib -MSQL::Abstract -E '
      say "version " . $SQL::Abstract::VERSION;
      package X {
        use parent "SQL::Abstract";
        use mro "c3";
        sub _where_field_IN { shift->next::method(@_) }
      }
      say X->new->select( table => ["*"], { foo => { -in => [] } } )
    '
    version 1.9002
    SELECT * FROM table WHERE 0=1

to

    $ perl -Ilib -MSQL::Abstract -E '
      say "version " . $SQL::Abstract::VERSION;
      package X {
        use parent "SQL::Abstract";
        use mro "c3";
        sub _where_field_IN { shift->next::method(@_) }
      }
      say X->new->select( table => ["*"], { foo => { -in => [] } } )
    '
    version 1.9003
    SELECT * FROM table WHERE foo IN ( )

and resulting in illegal SQL.

This was breaking behaviour that resulted in downstream issues affecting
at least SQL::Abstract::More
(see damil/SQL-Abstract-More#20).

Likewise, a class overriding `_where_field_BETWEEN` was not triggering
the parameter validation that disallowed empty array references or
undefined values to be used as parameters to `-between` and its
negation:

    $ perl -Ilib -MSQL::Abstract -E '
      say "version " . $SQL::Abstract::VERSION;
      package X {
        use parent "SQL::Abstract";
        use mro "c3";
        sub _where_field_BETWEEN { shift->next::method(@_) }
      }
      say X->new->select( table => ["*"], { foo => { -between => [] } } )
    '
    version 1.9003
    SELECT * FROM table WHERE ( foo BETWEEN AND )

This change ensures that the path taken by subclasses that override
those methods triggers equivalent code paths to those that don't.

Fixes https://rt.cpan.org/Ticket/Display.html?id=142341
@jjatria jjatria changed the title Handle empty array argument to in in subclasses Handle empty arrays in -in and -between in subclasses Sep 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant