The Wayback Machine - https://web.archive.org/web/20210124105125/https://github.com/sql-js/sql.js/pull/199
Skip to content

/ sql.js

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

Jump to bottom

Enabling SQLite FTS5 module by default. #199

Open
wants to merge 1 commit into
base: master
from
Open

Enabling SQLite FTS5 module by default. #199

wants to merge 1 commit into from

Conversation

@serpulga
Copy link

@serpulga serpulga commented May 15, 2017

Compiled using EMSCRIPTEN 1.37.9.

Tests:

$ ./test/run.sh 
Testing ./test/test_blob.js...	writeStringToMemory is deprecated and should not be called! Use stringToUTF8() instead!
Passed.
Testing ./test/test_database.js...	writeStringToMemory is deprecated and should not be called! Use stringToUTF8() instead!
Passed.
Testing ./test/test_errors.js...	writeStringToMemory is deprecated and should not be called! Use stringToUTF8() instead!
Passed.
Testing ./test/test_extension_functions.js...	writeStringToMemory is deprecated and should not be called! Use stringToUTF8() instead!
Passed.
Testing ./test/test_functions.js...	writeStringToMemory is deprecated and should not be called! Use stringToUTF8() instead!
Passed.
Testing ./test/test_issue128.js...	writeStringToMemory is deprecated and should not be called! Use stringToUTF8() instead!
Passed.
Testing ./test/test_issue55.js...	Passed.
Testing ./test/test_issue73.js...	writeStringToMemory is deprecated and should not be called! Use stringToUTF8() instead!
Passed.
Testing ./test/test_issue76.js...	Passed.
Testing ./test/test_node_file.js...	writeStringToMemory is deprecated and should not be called! Use stringToUTF8() instead!
Passed.
Testing ./test/test_statement.js...	writeStringToMemory is deprecated and should not be called! Use stringToUTF8() instead!
Passed.
Testing ./test/test_transactions.js...	writeStringToMemory is deprecated and should not be called! Use stringToUTF8() instead!
Passed.
Testing ./test/test_worker.js...	writeStringToMemory is deprecated and should not be called! Use stringToUTF8() instead!

assert.js:90
  throw new assert.AssertionError({
  ^
AssertionError: Reading BLOB
    at Worker.worker.onmessage (/Users/serpulga/Documents/src/sql.js/test/test_worker.js:23:14)
    at ChildProcess.<anonymous> (/Users/serpulga/Documents/src/sql.js/node_modules/workerjs/index.js:25:38)
    at emitTwo (events.js:87:13)
    at ChildProcess.emit (events.js:172:7)
    at handleMessage (internal/child_process.js:695:10)
    at Pipe.channel.onread (internal/child_process.js:440:11)
Fail!\e[0m
Warning\e[0m : 12 tests passed out of 13

Same error I get on master [6dc4ac5].
@serpulga
Enabling SQLite FTS5 module by default.
Loading status checks…
7e7ce61 
Compiled using EMSCRIPTEN 1.37.9.
@petermolnar
Copy link

@petermolnar petermolnar commented Jan 15, 2018

+1 for this; if the FTS5 engine is on, that'd allow search indexes created by other languages to be used in JS in the browser.
@lovasoa
Copy link
Member

@lovasoa lovasoa commented Jan 15, 2018

sql.js is already quite a huge dependency to have. I don't think adding other modules by default is a good idea.

But compiling different versions for different needs is a good idea.
@3CE8D2BAC65BDD6AA9
Copy link

@3CE8D2BAC65BDD6AA9 3CE8D2BAC65BDD6AA9 commented Mar 9, 2018 •
edited

I believe I am able to have FTS5 running. Here are the procedures:

  1. modify Makefile by adding "-DSQLITE_ENABLE_FTS5" in the CFLAGS line. So, the complete CFLAGS line will become CFLAGS=-O2 -DSQLITE_OMIT_LOAD_EXTENSION -DSQLITE_DISABLE_LFS -DLONGDOUBLE_TYPE=double -DSQLITE_THREADSAFE=0 -DSQLITE_ENABLE_FTS3 -DSQLITE_ENABLE_FTS3_PARENTHESIS -DSQLITE_ENABLE_FTS5
  2. make
  3. A few .js files will be created at the js directory. I tested worker.sql.js only so far
  4. In order to test it, I cloned http://kripken.github.io/sql.js/GUI/ to my server and then delete the line <script src="codemirror/mode/sql/sql.js"></script>
  5. Modify gui.js to point to my own worker.sql.js at this line <a href="https://github.com/kripken/sql.js"> to var worker = new Worker("worker.sql.js");
  6. run the gui page
  7. I typed the commands from http://www.sqlitetutorial.net/sqlite-full-text-search/ into the textarea box. All FTS5 commands work perfectly.

In the next few weeks, I will try to test the performance of FTS5 by adding thousands of documents.
@serpulga
Copy link
Author

@serpulga serpulga commented Mar 9, 2018

@3CE8D2BAC65BDD6AA9 Thank you for posting the steps. I was also able to run FTS5 but this pull request was to enable it by default inside sql.js; but since I agree with @lovasoa (we shouldn't add modules that most people won't use) I didn't even try to fix the failing unit tests.
@brodybits
brodybits reviewed Mar 9, 2018
View changes
Makefile
@@ -4,7 +4,7 @@ EMSCRIPTEN?=/usr/bin

EMCC=$(EMSCRIPTEN)/emcc

CFLAGS=-DSQLITE_OMIT_LOAD_EXTENSION -DSQLITE_DISABLE_LFS -DLONGDOUBLE_TYPE=double -DSQLITE_INT64_TYPE="long long int" -DSQLITE_THREADSAFE=0 -DSQLITE_ENABLE_FTS3 -DSQLITE_ENABLE_FTS3_PARENTHESIS
CFLAGS=-DSQLITE_OMIT_LOAD_EXTENSION -DSQLITE_DISABLE_LFS -DLONGDOUBLE_TYPE=double -DSQLITE_INT64_TYPE="long long int" -DSQLITE_THREADSAFE=1 -DSQLITE_ENABLE_FTS3 -DSQLITE_ENABLE_FTS3_PARENTHESIS -DSQLITE_ENABLE_FTS5

This comment has been minimized.

Sign in to view
@brodybits

brodybits Mar 9, 2018
Contributor

Why change the SQLITE_THREADSAFE default setting? Was this intended?

This comment has been minimized.

Sign in to view
@3CE8D2BAC65BDD6AA9

3CE8D2BAC65BDD6AA9 Mar 10, 2018

I noticed the same thing but no clue why there is such a different.

@kimown kimown mentioned this pull request Apr 16, 2018
Closed
@dinedal
Copy link
Collaborator

@dinedal dinedal commented Aug 2, 2018

I am open to the idea of making such compile time extensions into different versions of sql.js (such as memory-growth, debug, etc).

Would that be a good solution here? Can we expect future extensions to pass the standard test suite? Would a standard procedure for adding such extensions help in the future?
@pamelafox pamelafox mentioned this pull request Sep 19, 2018
Merged
@lovasoa lovasoa force-pushed the sql-js:master branch from b3211a6 to e902ec2 Feb 29, 2020
@lovasoa lovasoa force-pushed the sql-js:master branch from 351ffdd to 6e7ec33 Oct 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brodybits brodybits

@3CE8D2BAC65BDD6AA9 3CE8D2BAC65BDD6AA9

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
@serpulga @petermolnar @lovasoa @3CE8D2BAC65BDD6AA9 @dinedal @brodybits