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

Fix sqlite 3.41.0 build failed #12

Merged
merged 1 commit into from Feb 27, 2023

Conversation

peeweep
Copy link

@peeweep peeweep commented Feb 26, 2023

data/db/english/english.awk run failed with sqlite 3.41.0, the error
message is:

gawk -f ./english.awk ./wordlist | /usr/bin/sqlite3 english.db || \
	( rm -f english.db ; exit 1 )
Parse error near line 5: no such column: the
  INSERT INTO english (word, freq) VALUES ("the", "6.510891");
                             error here ---^

As sqlite 3.41.0 release note say:

The double-quoted string misfeature is now disabled by default for CLI builds. Legacy use cases can reenable the misfeature at run-time using the ".dbconfig dqs_dml on" and ".dbconfig dqs_ddl on" commands.

We should change this double quote to single quote

Ref: https://bugs.gentoo.org/896366

@peeweep peeweep force-pushed the fix-sqlite-3.41.0-dblquote branch 2 times, most recently from b667324 to c616333 Compare February 26, 2023 15:41
@epico
Copy link
Member

epico commented Feb 27, 2023

Could you format the commit message within 80 columns?

@peeweep
Copy link
Author

peeweep commented Feb 27, 2023

Reformatted.

@epico
Copy link
Member

epico commented Feb 27, 2023

I think it is also okay to remove the compile error from the commit message.

Otherwise this patch looks good to me.

@peeweep
Copy link
Author

peeweep commented Feb 27, 2023

Ok, I also think it can be removed, gentoo bugs provides all the error messages.

@epico
Copy link
Member

epico commented Feb 27, 2023

Sorry, I missed one thing.

How do you think to use ('%s', %f); ?

@peeweep
Copy link
Author

peeweep commented Feb 27, 2023

I think I also miss that, float don't need quote here.

As sqlite 3.41.0 release note say:

 The double-quoted string misfeature is now disabled by default for CLI
 builds. Legacy use cases can reenable the misfeature at run-time using
 the ".dbconfig dqs_dml on" and ".dbconfig dqs_ddl on" commands.

We should change this double quote to single quote

Ref: https://bugs.gentoo.org/896366
Signed-off-by: jinqiang zhang <peeweep@0x0.ee>
@epico epico merged commit ffe471c into ibus:main Feb 27, 2023
@peeweep
Copy link
Author

peeweep commented Feb 27, 2023

Thanks for review!

@peeweep peeweep deleted the fix-sqlite-3.41.0-dblquote branch February 27, 2023 03:52
@epico
Copy link
Member

epico commented Feb 27, 2023

Thanks for the patch!

@hosiet
Copy link

hosiet commented Sep 3, 2023

     # Insert data into english table
-    {   printf "INSERT INTO english (word, freq) VALUES (\"%s\", \"%f\");\n", $1, $2}
+    {   printf "INSERT INTO english (word, freq) VALUES (\'%s\', %f);\n", $1, $2}

@peeweep @epico I found the extra backslash in this pull request to be problematic. When being executed by mawk, it will complain with the following error message:

Parse error near line 20004: unrecognized token: "\"
  INSERT INTO english (word, freq) VALUES (\'falkland\', 0.000208);
                             error here ---^
make[5]: *** [Makefile:601: english.db] Error 1

When processed with gawk, no error will appear.

I believe these backslashes are unnecessary and harmful. Could we drop them? After removing the backslashes, the build will success with either gawk or mawk.

      # Insert data into english table
 -    {   printf "INSERT INTO english (word, freq) VALUES (\"%s\", \"%f\");\n", $1, $2}
-+    {   printf "INSERT INTO english (word, freq) VALUES (\'%s\', %f);\n", $1, $2}
++    {   printf "INSERT INTO english (word, freq) VALUES ('%s', %f);\n", $1, $2}

@epico
Copy link
Member

epico commented Sep 5, 2023

@hosiet Okay, fixed. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants