Skip to content

Correction of multiline UPDATE statements in the Postgres driver#2629

Closed
therufa wants to merge 7 commits intobcit-ci:developfrom
therufa:develop
Closed

Correction of multiline UPDATE statements in the Postgres driver#2629
therufa wants to merge 7 commits intobcit-ci:developfrom
therufa:develop

Conversation

@therufa
Copy link
Copy Markdown

@therufa therufa commented Sep 9, 2013

Added support for returning values in multi-line UPDATE query statements to the PostgreSQL database driver

@therufa
Copy link
Copy Markdown
Author

therufa commented Sep 9, 2013

The feature is provided by the PostgreSQL engine, and it would be great to be able to use this feature without any hack in CI.

Source:
http://www.postgresql.org/docs/9.2/static/sql-update.html#AEN79130

@narfbg
Copy link
Copy Markdown
Contributor

narfbg commented Sep 9, 2013

Nice one, although it needs some additional work:

  • Please check our styleguide, use tabs for indentation and remove the empty line at EOF.
  • Add a changelog entry.
  • I haven't checked, but if INSERT and UPDATE support the RETURNING clause, doesn't REPLACE do as well?
  • Furthermore, is it SET that you're also adding to or UPDATE ... SET ?

@therufa
Copy link
Copy Markdown
Author

therufa commented Sep 9, 2013

I do hope, that I could accomplish everything properly.

By the way; there is no REPLACE statement in the PostgreSQL.
If I do understand your question right, yes I wanted the SET at that location :)

@therufa
Copy link
Copy Markdown
Author

therufa commented Sep 10, 2013

i hope everything is right now

@narfbg
Copy link
Copy Markdown
Contributor

narfbg commented Sep 10, 2013

So, you're telling me that you can do something like this?

SET time zone 'Europe/Sofia' RETURNING '2';

This seems quite odd to me. :)
Otherwise ... the PR is not complete, but I'll do the afterwork so that it doesn't take days in explanations between the two of us. You'll see what I mean afterwards.

@therufa
Copy link
Copy Markdown
Author

therufa commented Sep 10, 2013

Well, the set part might make no sense with your example, but if you work with multi-line queries it can be quite annoying that the SET keyword turns the query to a non-reading type.

The example of @fema90 shows exactly the problem. Look at the second line; that SET there would break the whole query, and the result would be that you have no result, but that the query went well. Thats not what I want in such a case.

I hope this explains my step. :)

@therufa
Copy link
Copy Markdown
Author

therufa commented Sep 10, 2013

So I think the current solution fits everything, except what you've mentioned (to be honest I don't really understand what you've meant, with the PR, I hope that is not too much).

narfbg added a commit that referenced this pull request Sep 10, 2013
An improved version of PR #2629.
Also removes REPLACE from the regular expression, as it is not supported by PostgreSQL.
@narfbg
Copy link
Copy Markdown
Contributor

narfbg commented Sep 10, 2013

@fema90's example shows an UPDATE query, the regular expression that I asked you about would validate a query that bebings with a SET. The one now, after your last commit is even messier. :)

See the above commit for the cleaner way. Sorry for making you add a changelog entry btw, I just realized it was the same bug that was fixed for INSERT, so no need to have 2 lines in there.

@narfbg narfbg closed this Sep 10, 2013
@therufa
Copy link
Copy Markdown
Author

therufa commented Sep 11, 2013

Well, this does not solve the base problem (at least not "mine"), but maybe I just don't understand something, and maybe its off topic:
Should I pass only single-line queries to the db->query() method?

In the Postgres community (as it is in the Oracle and in the MsSQL) its common practice to write long queries, and for readability these are split into multiple lines.

So wouldn't it make sense to support this practice, or is it considered as wrong? I'm just asking right now.

narfbg added a commit that referenced this pull request Sep 11, 2013
@narfbg
Copy link
Copy Markdown
Contributor

narfbg commented Sep 11, 2013

The good old PCRE ignoring everything after the newline character, even without the 'm' modifier ... should be OK now. :)

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

Successfully merging this pull request may close these issues.

2 participants