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

Change conditional helpers' method signature #193

Open
yanickrochon opened this issue Mar 10, 2020 · 0 comments
Open

Change conditional helpers' method signature #193

yanickrochon opened this issue Mar 10, 2020 · 0 comments

Comments

@yanickrochon
Copy link

yanickrochon commented Mar 10, 2020

Problem

The current implementation makes it impossible to implement the $regex conditional helper. From the documentation :

{ <field>: { $regex: /pattern/, $options: '<options>' } }
{ <field>: { $regex: 'pattern', $options: '<options>' } }
{ <field>: { $regex: /pattern/<options> } }

I tried implementing it with :

builder.conditionalHelpers.add('$regex', (column, value) => column + " REGEXP " + value);
builder.conditionalHelpers.add('$options', (_, __, values) => {
   values.pop();  // remove current value from array
});


const sql = builder.sql({
   type: 'delete'
   , table: 'users'
   , where: {
      created_at: { $regex: 5, $options: 'i' }
   }
});
{
  query: 'delete from "users" where "users"."created_at" REGEXP $1 and',
  values: [ 5 ],
  original: {
    type: 'delete',
    table: 'users',
    where: { created_at: [Object] },
    __defaultTable: 'users'
  },
  toString: [Function],
  toQuery: [Function]
}

Notice how the question ends with and (invalid SQL syntax) and how I am completely unable to process the "ignore case" option.

Solution

In the code, the helper function is invoked like this :

helpers.get(key).fn(
column
, where[key] === null ? null : utils.parameterize(where[key], values)
, values
, table
, where[key]
)

Passing (arguably) the same value where[key] in two different arguments. The function helper does not need where[key], however it does need where.

Therefore, if the helper function was called like this :

helpers.get(key).fn(
   column
 , where[key] === null ? null : utils.parameterize(where[key], values)
 , values
 , table
 , where
)

The entire filter context would be available, including the helper value. Such as

builder.conditionalHelpers.add('$regex', (column, value, _, filter) => {
   // NOTE : filter.$regex is the same as before; we know what the 'key' value is!

   const isCaseInsensitive = checkOptions(filter.$options);
   if (isCaseInsensitive) {
      return "LOWER(" + column + ") REGEXP " + value;
   } else {
      return column + " REGEXP " + value;
   }
});

Also

The values should not be pushed to the array before the helper function returns, and the helper function returned value should not be undefined. For example :

helpers.get(key).fn(
   column
 , where[key] === null ? null : utils.parameterize(where[key], values)
 , values
 , table
 , where
)

So, the above implementation should not produce

 {
query: 'delete from "users" where "users"."created_at" REGEXP $1 and',
  values: [ 'test', 'i' ],
  ...
}

but should produce

 {
query: 'delete from "users" where LOWER("users"."created_at") REGEXP $1 ,
  values: [ 'test' ],
  ...
}

That is, ignore $options completely, and do not push it's value to the values list.

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

No branches or pull requests

1 participant