Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

fix: use official math functions #619

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

Open
BobdenOs wants to merge 1 commit into sql-js:master
base: master
Choose a base branch
Loading
from BobdenOs:master
Open

Conversation

Copy link

@BobdenOs BobdenOs commented Sep 17, 2025

The currently used function extension file is listed by SQLite as "not supported nor maintained" and "Use At Your Own Risk" (docs).

The current feature flag SQLITE_ENABLE_MATH_FUNCTIONS is enabled automatically by newly generated Makefiles. Which provides an maintained and documented version of the math functions provided from the originally contributed functions extension.

Copy link
Member

lovasoa commented Sep 17, 2025
edited
Loading

is the functionality exactly equivalent? Or will this break existing applications that use the old functions?

Copy link
Author

I compared the extension-functions.c function definition with the official documentation. Which shows that it would definitely be a breaking change. It also shows that the current state of sql.js is missing some SQL specified math functions. I didn't find any reported issues that noticed that the handful of math functions are missing.

missing in SQLITE_ENABLE_MATH_FUNCTIONS

aliases

  • atn2 (atan2 is official)
  • square (sqrt is official)
  • charindex (instr is official)

functionality

scalar

  • cot
  • coth
  • replicate
  • difference
  • sign
  • leftstr
  • rightstr
  • reverse
  • proper
  • padl
  • padr
  • padc
  • strfilter

aggregate

  • stdev
  • variance
  • mode
  • median
  • lower_quartile
  • upper_quartile

missing in extension-functions.c

aliases

  • ceiling
  • pow

functionality

  • ln
  • log(B,X) (only has log(X))
  • log2
  • mod
  • trunc

Copy link
Member

lovasoa commented Sep 17, 2025
edited
Loading

Looks like extension functions.c is better, right?

Some of the missing features in MATH_FUNCTIONS would be hard to reproduce, whereas things like mod or log bases are easy to reproduce.

Copy link
Author

It greatly depends on the goal of the package. The name being sql.js and not something containing SQLite means that it is totally up to you to decide what is the correct behavior.

The trigger for me to make this PR is that we have a test suite that checks that our databases support the SQL specification. We currently are using better-sqlite3 for the node runtime, but would also like to support other runtimes by using sql.js as a substitute. With these missing functions being the only gap in our test suite.

Probably the best solution would be to support all functions, but the feature flag and the extension define the same functions. I am not sure whether that will cause compilation problems. 🤷

It is also not even a show stopper for our project. As we know the difference in behavior between the implementations and can either opt for documenting the missing functions or decide to use create_function to polyfill the missing functions.

Copy link
Member

lovasoa commented Sep 17, 2025

Yes, I think create_function is the way to go for you ! It looks like it's much easier to fill the gap from extension-functions.c to SQLITE_ENABLE_MATH_FUNCTIONS than the opposite.

BobdenOs reacted with thumbs up emoji

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2 participants

AltStyle によって変換されたページ (->オリジナル) /