Skip to content

upstream 3.21.0 with UPDATE/DELETE LIMIT support #500

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

Closed
wants to merge 1 commit into from
Closed

upstream 3.21.0 with UPDATE/DELETE LIMIT support #500

wants to merge 1 commit into from

Conversation

temoto
Copy link

@temoto temoto commented Nov 29, 2017

Amalgamation built with

CFLAGS='-std=gnu99 -DSQLITE_ENABLE_RTREE -DSQLITE_THREADSAFE=1 -DSQLITE_ENABLE_FTS3 -DSQLITE_ENABLE_FTS3_PARENTHESIS -DSQLITE_ENABLE_FTS4_UNICODE61 -DSQLITE_TRACE_SIZE_LIMIT=15 -DSQLITE_DISABLE_INTRINSIC -Wno-deprecated-declarations' ./configure --enable-rtree --enable-fts3 --enable-fts4 --enable-json1 --enable-update-limit

maybe defining flags before configure was too much, sorry for mess.

Originally proposed in
#213
#215

@temoto temoto closed this Nov 29, 2017
@temoto temoto deleted the enable-update-limit branch November 29, 2017 19:55
@temoto temoto restored the enable-update-limit branch November 29, 2017 19:55
@temoto
Copy link
Author

temoto commented Nov 29, 2017

Sorry, I merged and deleted branch in my repo. This patch is still relevant.

@temoto temoto reopened this Nov 29, 2017
@coveralls
Copy link

coveralls commented Nov 29, 2017

Coverage Status

Coverage remained the same at 68.639% when pulling bafdab8 on temoto:enable-update-limit into d5ffb5c on mattn:master.

5 similar comments
@coveralls
Copy link

Coverage Status

Coverage remained the same at 68.639% when pulling bafdab8 on temoto:enable-update-limit into d5ffb5c on mattn:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 68.639% when pulling bafdab8 on temoto:enable-update-limit into d5ffb5c on mattn:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 68.639% when pulling bafdab8 on temoto:enable-update-limit into d5ffb5c on mattn:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 68.639% when pulling bafdab8 on temoto:enable-update-limit into d5ffb5c on mattn:master.

@coveralls
Copy link

coveralls commented Nov 29, 2017

Coverage Status

Coverage remained the same at 68.639% when pulling bafdab8 on temoto:enable-update-limit into d5ffb5c on mattn:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 68.639% when pulling bafdab8 on temoto:enable-update-limit into d5ffb5c on mattn:master.

5 similar comments
@coveralls
Copy link

Coverage Status

Coverage remained the same at 68.639% when pulling bafdab8 on temoto:enable-update-limit into d5ffb5c on mattn:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 68.639% when pulling bafdab8 on temoto:enable-update-limit into d5ffb5c on mattn:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 68.639% when pulling bafdab8 on temoto:enable-update-limit into d5ffb5c on mattn:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 68.639% when pulling bafdab8 on temoto:enable-update-limit into d5ffb5c on mattn:master.

@coveralls
Copy link

coveralls commented Nov 29, 2017

Coverage Status

Coverage remained the same at 68.639% when pulling bafdab8 on temoto:enable-update-limit into d5ffb5c on mattn:master.

@mattn
Copy link
Owner

mattn commented Nov 30, 2017

You can use json1 with go build -tag json1

@mattn mattn closed this Nov 30, 2017
@mattn mattn reopened this Nov 30, 2017
@mattn
Copy link
Owner

mattn commented Nov 30, 2017

please drop json1 feature from this PR

@coveralls
Copy link

coveralls commented Nov 30, 2017

Coverage Status

Coverage remained the same at 68.639% when pulling bafdab8 on temoto:enable-update-limit into d5ffb5c on mattn:master.

@mattn
Copy link
Owner

mattn commented Nov 30, 2017

please drop updating of sqlite3-binding.c and #cgo CFLAGS: -DSQLITE_ENABLE_JSON1.

@temoto
Copy link
Author

temoto commented Dec 1, 2017

@mattn done

@coveralls
Copy link

coveralls commented Dec 1, 2017

Coverage Status

Coverage remained the same at 68.639% when pulling 51aeffa on temoto:enable-update-limit into d5ffb5c on mattn:master.

@temoto temoto changed the title upstream 3.21.0 with json1 and UPDATE/DELETE LIMIT support upstream 3.21.0 with UPDATE/DELETE LIMIT support Dec 1, 2017
@temoto temoto closed this Dec 2, 2017
@temoto temoto deleted the enable-update-limit branch December 2, 2017 17:04
@temoto temoto restored the enable-update-limit branch December 2, 2017 17:04
@temoto
Copy link
Author

temoto commented Dec 2, 2017

Sorry I have script that deletes branches after merge.

Ping. Is everything OK?

@temoto temoto reopened this Dec 2, 2017
@coveralls
Copy link

coveralls commented Dec 2, 2017

Coverage Status

Coverage remained the same at 68.639% when pulling 51aeffa on temoto:enable-update-limit into d5ffb5c on mattn:master.

1 similar comment
@coveralls
Copy link

coveralls commented Dec 2, 2017

Coverage Status

Coverage remained the same at 68.639% when pulling 51aeffa on temoto:enable-update-limit into d5ffb5c on mattn:master.

@temoto
Copy link
Author

temoto commented Dec 5, 2017

@mattn ping

@mattn
Copy link
Owner

mattn commented Dec 5, 2017

please drop changes of sqlite3-binding.c. I think this is not required changes for it.

@temoto
Copy link
Author

temoto commented Dec 5, 2017

Amalgamation contains changes to query parser, adding order by and limit clause to update, delete statements. Without changes to sqlite3-binding.c it gives error:

update queue
  set locked = current_timestamp
  where (locked is null) and (scheduled < current_timestamp)
  order by scheduled limit ?

-- near "order": syntax error`

@temoto
Copy link
Author

temoto commented Dec 5, 2017

https://www.sqlite.org/compile.html

SQLITE_ENABLE_UPDATE_DELETE_LIMIT
This option enables an optional ORDER BY and LIMIT clause on UPDATE and DELETE statements.
If this option is defined, then it must also be defined when using the 'lemon' tool to generate a parse.c file. Because of this, this option may only be used when the library is built from source, not from the amalgamation or from the collection of pre-packaged C files provided for non-Unix like platforms on the website.

@mattn
Copy link
Owner

mattn commented Dec 5, 2017

Well, 3.21.0 is already merged in master branch. c41df79

Where did you download from?

@temoto
Copy link
Author

temoto commented Dec 5, 2017

I'm not saying there was version change, I just compiled amalgamation with update-limit flag.

./configure --enable-rtree --enable-fts3 --enable-fts4 --enable-update-limit
make sqlite3.c

Code is taken from here http://www.sqlite.org/2017/sqlite-src-3210000.zip
Snapshop of the complete (raw) source tree for SQLite version 3.21.0. See How To Compile SQLite for usage details.
(sha1: 1892ebbac215095351a0ecc0c60a36e4d03104bc)

@mattn
Copy link
Owner

mattn commented Dec 6, 2017

sqlite3-binding.c have some #ifdef SQLITE_ENABLE_UPDATE_DELETE_LIMIT. So if it doesn't work, something should be wrong.

@temoto
Copy link
Author

temoto commented Dec 6, 2017

Please pay attention to this quote by sqlite developers.

If this option is defined, then it must also be defined when using the 'lemon' tool to generate a parse.c file. Because of this, this option may only be used when the library is built from source, not from the amalgamation or from the collection of pre-packaged C files provided for non-Unix like platforms on the website.

Amalgamation source they provide was configured without this flag. So it contains ifdefs to have update/delete order/limit feature to work, but it does not contain parser code to allow order/limit words in SQL statements. Because parser code is finished after make sqlite3.c.

Yes, it could be better for sqlite devs to add something like

#ifdef SQLITE_ENABLE_UPDATE_DELETE_LIMIT
#error must configure with --enable-update-limit
#endif

I will try to send this patch, but it's only helpful for debugging the problem. We don't have a problem like "we don't know why it doesn't build". We know why it doesn't build, we know how to make it build and we know how to make it work.

@temoto
Copy link
Author

temoto commented Dec 6, 2017

sqlite-src-321000/autoconf/README.txt

Other preprocessor defines

Additionally, preprocessor defines may be specified by using the OPTS macro
on the NMAKE command line. However, not all possible preprocessor defines
may be specified in this manner as some require the amalgamation to be built
with them enabled (see http://www.sqlite.org/compile.html). For example, the
following will work:
"OPTS=-DSQLITE_ENABLE_STAT4=1 -DSQLITE_ENABLE_JSON1=1"
However, the following will not compile unless the amalgamation was built
with it enabled:
"OPTS=-DSQLITE_ENABLE_UPDATE_DELETE_LIMIT=1"

@mattn
Copy link
Owner

mattn commented Dec 6, 2017

Thanks your pointing this. I understood now. However, please take a while for me. I hope that I'll change sqlite3-binding.c in my self.

@mattn
Copy link
Owner

mattn commented Dec 6, 2017

sqlite3 amalgamation code is large for reviewing changes. If, in the future, someone send me a pull-request of changes on sqlite3-binding.c, I may not be able to review it correctly. If someone(not you) is malicious, I might merge something. So I want that I'll change sqlite3-binding.c in my self. Sorry.

@temoto
Copy link
Author

temoto commented Dec 6, 2017

I understand of course. Instead of reviewing changes you can just regenerate amalgamation from official sqlite source. I can patch your upgrade tool if you like.

@gjrtimmer
Copy link
Collaborator

Should be closed because of #564

@mattn
Copy link
Owner

mattn commented May 24, 2018

Yes, but note that not mean that this enabling delete limit also when using libsqlite3 on system always.

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.

4 participants