Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(863)

Issue 8899012: Put debugging assertions into sql::Statement. (Closed)

Created:
9 years ago by Scott Hess - ex-Googler
Modified:
9 years ago
CC:
chromium-reviews, Paweł Hajdan Jr.
Visibility:
Public.

Description

Put debugging assertions into sql::Statement. Pulls out the core of gbillock's http://codereview.chromium.org/8283002/ - Move NOTREACHED and similar checks into the sql:: implementation code. - Add malformed SQL checks to Connection::Execute. - Add SQL-checking convenience methods to Connection. The general idea is that the sql:: framework assumes valid statements, rather than having client code contain scattered ad-hoc (and thus inconsistent) checks. This version puts back Statement operator overloading and loosy-goosy Execute() calls to allow other code to be updated in small batches. R=gbillock@chromium.org,jhawkins@chromium.org,dhollowa@chromium.org BUG=none TEST=sql_unittests,unit_tests:*Table*.* Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=114118

Patch Set 1 #

Total comments: 1

Patch Set 2 : Oops, Execute() should stay loose. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+180 lines, -138 lines) Patch
M sql/connection.h View 4 chunks +13 lines, -3 lines 0 comments Download
M sql/connection.cc View 1 17 chunks +38 lines, -28 lines 0 comments Download
M sql/connection_unittest.cc View 5 chunks +20 lines, -4 lines 0 comments Download
M sql/meta_table.h View 1 chunk +1 line, -1 line 0 comments Download
M sql/meta_table.cc View 4 chunks +4 lines, -17 lines 0 comments Download
M sql/sqlite_features_unittest.cc View 1 chunk +2 lines, -1 line 0 comments Download
M sql/statement.h View 1 5 chunks +28 lines, -6 lines 0 comments Download
M sql/statement.cc View 8 chunks +74 lines, -74 lines 0 comments Download
M sql/statement_unittest.cc View 1 chunk +0 lines, -4 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
Scott Hess - ex-Googler
The original review of all of this is really on the referenced CL. But Greg ...
9 years ago (2011-12-09 22:54:40 UTC) #1
James Hawkins
LGTM
9 years ago (2011-12-10 02:54:37 UTC) #2
Greg Billock
On 2011/12/10 02:54:37, James Hawkins wrote: > LGTM LGTM Thanks, Scott. I was going to ...
9 years ago (2011-12-12 19:14:55 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/shess@chromium.org/8899012/12
9 years ago (2011-12-12 22:41:29 UTC) #4
commit-bot: I haz the power
9 years ago (2011-12-12 23:51:01 UTC) #5
Change committed as 114118

Powered by Google App Engine
This is Rietveld 408576698