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

Binding variables conflicts with some Postgres SQL queries. #111

Closed
dhrrgn opened this issue Jan 22, 2016 · 7 comments
Closed

Binding variables conflicts with some Postgres SQL queries. #111

dhrrgn opened this issue Jan 22, 2016 · 7 comments

Comments

@dhrrgn
Copy link

dhrrgn commented Jan 22, 2016

Assume you are trying to execute the following SQL query.

SELECT id
FROM users
WHERE created_at >= ':createdDate 00:00:00'::TIMESTAMP
  AND created_at <= ':createdDate 23:59:59'::TIMESTAMP

The ::type syntax, in Postgres, casts the value to that type (above we are converting the string to a TIMESTAMP). Now, lets try to execute the query and bind a value:

$conn->fetchAll($sql, ['createdDate' => '2015/12/01']);

This gives you an error: "Undefined offset: 0" in src/Rebuilder.php on line 247.

@pmjones
Copy link
Member

pmjones commented Jan 22, 2016

Can you tell if #108 would resolve that?

@pmjones
Copy link
Member

pmjones commented Jan 22, 2016

Alternatively, would it be enough to have the interpolator ignore :: in what would otherwise be recognized as placeholders?

@dhrrgn
Copy link
Author

dhrrgn commented Jan 22, 2016

I can't tell just by looking at that particular PR (it is rather massive). Your second suggestion would probably be the best solution. I am not sure how the interpolator works (haven't looked), but it should be something equivalent to this regexp: :([\w.\-]+) (not sure if you want to allow periods and hyphens).

@pmjones
Copy link
Member

pmjones commented Jan 22, 2016

(/me nods) The code in question is in Rebuilder::rebuildPart(). If you want to submit a PR with a test I'll be happy to review, otherwise I'll come back around to this at some point in the future.

@dhrrgn
Copy link
Author

dhrrgn commented Jan 22, 2016

I will see what I can do. I won't be around this weekend, and don't have time to dig into it today, but I will take a look next week.

@dhrrgn
Copy link
Author

dhrrgn commented Jan 22, 2016

Also, sorry, I totally missed #104 when looking through the open issues to see if this was mentioned anywhere. I am actually just going to close this issue, and track that issue. if I come up with an easier solution than that massive pull request, I will submit that.

@dhrrgn dhrrgn closed this as completed Jan 22, 2016
pmjones pushed a commit that referenced this issue Feb 5, 2017
@pmjones
Copy link
Member

pmjones commented Feb 5, 2017

@dhrrgn Revisiting this after a long hiatus; this should be fixed on the 2.x branch now. Sorry for the very extended delay.

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

2 participants