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

sql() overeagerly converts dollar characters into parameter placeholders #96

Closed
sneakertack opened this issue Jun 25, 2017 · 5 comments
Closed

Comments

@sneakertack
Copy link
Contributor

@Suor (as the issue was discovered in the process of using PostgreSQL/sql-bricks-postgres - not sure where a fix if any ought to be applied)

sql(), recommended as a fallback for additional Postgres operations not covered by sql-bricks-postgres, will mangle values where an actual dollar character is desired.

In my case, I wanted to construct queries that do POSIX regular expression matching. The Postgres syntax looks something like:

SELECT * FROM table1 WHERE col1 ~* '.*someregex\d+$' (note the use of $ as an end-of-line anchor in the regex)

In such a case, doing sql.select().from('table1').where(sql("col1 ~* '.*someregex\d+$'")) will replace $ with $1 (in both toString() and toParams()).

Is there any way to escape the $-character to prevent it from being treated as a parameter placeholder? I looked at https://github.com/CSNW/sql-bricks/blob/master/sql-bricks.js#L69 and that line suggests that there isn't one at the moment.

@sneakertack
Copy link
Contributor Author

I've since realized that a proper way to achieve the above could be to do sql.select().from('table1').where(sql("col1 ~* $", '.*someregex\d+$'))

It could help to have the features implemented by #61 and #74 explicitly described in the doc pages though... would it be worth updating them? I don't mind writing a PR with documentation updates, but the gh-pages branch seems to have been untouched in a while, so I'm not entirely sure about the docs' current status.

@prust
Copy link
Collaborator

prust commented Jun 30, 2017

@sneakertack:

the gh-pages branch seems to have been untouched in a while, so I'm not entirely sure about the docs' current status.

You're right, the gh-pages branch wasn't updated in 2016, but there were doc fixes in 2016. I just now updated it to the latest (2.0.2).

I don't mind writing a PR with documentation updates

That would be much appreciated.

@prust
Copy link
Collaborator

prust commented Aug 5, 2017

@sneakertack: As mentioned above, there were missing docs added in 2016 (180314b) that needed to be pushed to gh-pages -- that was done on June 30.

Today I added some more documentation in 46e5c7b and 45eef1c, so I think this can be closed.

Regarding the request for a way to escape the '$' character... I suppose we could make '\$' and '\?' be an escape, via adding [^\\] to the beginning of all the regexes in toString()... but that would disallow a placeholder as the very first character passed to sql(). We might be able to allow start-of-string OR non-escape-character via ^|[^\\] -- but I haven't tested that to verify that it works. At this point, I'm hesitant to add the complexity unless there are more requests demonstrating the need for it.

@prust prust closed this as completed Aug 5, 2017
@Suor
Copy link
Contributor

Suor commented Aug 6, 2017

@prust the usual thing to use is double char like $$ here not \$. In case if you ever going to implement this)

@sneakertack
Copy link
Contributor Author

Actually $$ opens up another can of worms, because sometimes strings might be represented as $$a string$$ in Postgres 😬. Maybe a 3rd option we could not-implement would be to preserve dollar-literals encountered within quotes.

But yes I no longer have that strong a need for the escaping originally raised, especially not if it makes things more difficult - after all the "workaround" of parametrizing your query and moving $ or ?literals into the parametrized value makes for a safer query.

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

3 participants