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

SQLite: Revisit user defined functions registered by DBAL #5745

Closed
derrabus opened this issue Oct 11, 2022 · 7 comments
Closed

SQLite: Revisit user defined functions registered by DBAL #5745

derrabus opened this issue Oct 11, 2022 · 7 comments

Comments

@derrabus
Copy link
Member

Q A
New Feature yes
RFC yes

Summary

While working on #5742, I haven't questioned the three functions that we always register in SQLite, but maybe we should:

  • sqrt(): SQLite does have a function with that name already, but it might be unavailable depending on how SQLite is compiled.
  • mod(): Same issue. Apart from that, SQLite has a % operator that should always be available.
  • locate(): SQLite has a function called INSTR() which sounds like it's doing similar things.

Does the DBAL itself need those functions? Or why do we register those?

Looking at sqrt() and mod() specifically, we seem to compensate that SQLite might be compiled without mathematical functions. I don't know why anyone should do this, but who am I to judge? Anyway, if SQLite's mathematical functions are available, the DBAL actively overloads those with its own implementation, if I understood SQLite's API correctly. A consumer might expect to get SQLite's version of sqrt() while in fact DBAL's version is used. Not sure if that makes a big difference though.

Maybe it's a better idea to not preemptively register those functions and instead offer an optional middleware that polyfills mathematical functions.

@morozov
Copy link
Member

morozov commented Oct 11, 2022

If those functions can be made available in SQLite, I'd drop them from the DBAL. The DBAL doesn't have a goal of making all platforms identical, it provides the means for writing portable applications. If the users of SQLite use certain functions of SQLite, they should compile it accordingly.

I wouldn't even bother about the middleware. Once SQLite is properly installed on CI, its could be used as a reference.

@derrabus
Copy link
Member Author

I did some research in SQLite's change log. The mathematical functions are a recent addition (March 2021), so it might be too soon to deprecate our emulation. However, it still bothers me that we override those functions even if SQLite provides them.

With INSTR() it's a different story. It has existed for almost ten years now, so I think we can safely assume that it's available.

@morozov
Copy link
Member

morozov commented Oct 11, 2022

The mathematical functions are a recent addition (March 2021), so it might be too soon to deprecate our emulation.

The functions in question are quite exotic. I don't remember ever using them in SQL. Furthermore, those who need them but don't have them enabled natively still can emulate them via UDFs registered via a middleware.

Besides that, I don't think there was ever a demand to implement these functions in a portable way in the first place. Somebody may have implemented them just because it was technically possible.

@derrabus
Copy link
Member Author

Shall we just remove them in 4.0.x then? I'm unsure how to trigger a proper deprecation for mod() and sqrt().

@derrabus
Copy link
Member Author

We have deprecated our own usage of SQRT() in #4724. And MOD() can be switched to the % operator.

@morozov
Copy link
Member

morozov commented Oct 12, 2022

Shall we just remove them in 4.0.x then? I'm unsure how to trigger a proper deprecation for mod() and sqrt().

That sounds like the best we can do.

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants