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

Compile with Pdo\Sqlite::loadExtension() by default? #17251

Closed
stancl opened this issue Dec 24, 2024 · 10 comments
Closed

Compile with Pdo\Sqlite::loadExtension() by default? #17251

stancl opened this issue Dec 24, 2024 · 10 comments

Comments

@stancl
Copy link

stancl commented Dec 24, 2024

Description

Apologies if this isn't the right place to ask a question like this, but am I understanding correctly that compiling PHP with PDO_SQLITE_OMIT_LOAD_EXTENSION=0 adds a loadExtension() method to PDO\Sqlite, letting developers load SQLite extensions?

(Added in #12804)

See:

[AC_DEFINE([PDO_SQLITE_OMIT_LOAD_EXTENSION], [1],
#ifndef PDO_SQLITE_OMIT_LOAD_EXTENSION
/* Attempts to load an SQLite extension library. */
PHP_METHOD(Pdo_Sqlite, loadExtension)

I'm curious why this is omitted by default. The RFC clarifies that the approach used in the PR above only allows loading extensions using the C API, not SQLite statements, so there shouldn't be any risks?

Is it because the sqlite library on the server may be compiled with SQLITE_OMIT_LOAD_EXTENSION and the method has no way of knowing that?

Being able to load SQLite extensions in PDO, without recompiling PHP, would be very helpful imo since there's a decent number of questions online asking how to achieve certain things with PDO (which is often a requirement e.g. in Laravel's ORM) vs the SQLite3 class directly.

@SakiTakamachi
Copy link
Member

@kocsismate
Can you look it?

@cmb69
Copy link
Member

cmb69 commented Dec 24, 2024

If SQLite3 doesn't have sqlite3_load_extension(), PDO_SQLITE_OMIT_LOAD_EXTENSION is defined (to 1), and then Pdo\Sqlite::loadExtension() is not available. If you use an SQLite3 which has sqlite3_load_extension(), the PHP method is available.

@cmb69 cmb69 closed this as not planned Won't fix, can't repro, duplicate, stale Dec 24, 2024
@stancl
Copy link
Author

stancl commented Dec 24, 2024

@cmb69 Wouldn't that check be running in the environment compiling the PHP build, not necessarily the environment running it? Not sure how you approach these things, but since most people install pre-built PHP, the environment compiling it shouldn't necessarily mean much here I think?

I'm mainly wondering if there could be a runtime check in Pdo\Sqlite::loadExtension() rather than a compile time check essentially.

@kocsismate
Copy link
Member

@stancl Yes, the check runs during compile time.

I'm mainly wondering if there could be a runtime check in Pdo\Sqlite::loadExtension() rather than a compile time check essentially.

The only thing we could do in this case is to trigger an exception during run-time if the necessary dependency (sqlite3_load_extension()) is not available. So I guess the best course of action would be to ask packagers to allow this sqlite3 function in their build.

@cmb69
Copy link
Member

cmb69 commented Dec 24, 2024

The only thing we could do in this case is to trigger an exception during run-time if the necessary dependency (sqlite3_load_extension()) is not available.

Can we? Sure, we could use runtime linkinkg (use dlopen() and friends) to check whether the symbol is available, but I wouldn't go down that path. And actually, we're doing similar compile-time checks for a lot of dependency libraries. Plus, SQLite3 supports the load_extension() SQL function, which might be usable.

@stancl
Copy link
Author

stancl commented Dec 25, 2024

Sorry if this is a dumb question, but does sqlite3_load_extension have to be available for the compilation to work at all, or does compilation work regardless and it's just that the compilation step tries to make the PHP functionality match the linked sqlite library?

If it's the latter, I wonder if there really is value in conditional compilation. If I can download a binary compiled for sqlite with sqlite3_load_extension but I don't have it in my local sqlite, I'll just get an exception. If I download a binary compiled for sqlite WITHOUT that feature and I do have it in my sqlite, I'll miss out on functionality just because of what the packager's environment was.

Runtime exceptions that decouple the compilation environment from the runtime environment would make more sense to me, though again it's very possible I'm missing something.

@cmb69
Copy link
Member

cmb69 commented Dec 26, 2024

It's not so much about compilation, but about the linking. If you use a function, during linking the linker will complain that the function is not defined. If you run against a shared library, you could use runtime linking, which avoids the problem that the linker complains, but that is somewhat brittle, and won't work if php_sqlite3/php_pdo_sqlite are build against a static libsqlite3.

@stancl
Copy link
Author

stancl commented Dec 26, 2024

Ah I see, didn't realize it wasn't being linked at runtime. So I'm guessing if the compilation environment doesn't match the actual runtime environment, that's just kind of unfortunate but a problem of the person using the pre-built version? Meaning if they have a somewhat nonstandard setup, it's best if they just compile PHP themselves?

@cmb69
Copy link
Member

cmb69 commented Dec 26, 2024

Meaning if they have a somewhat nonstandard setup, it's best if they just compile PHP themselves?

Right. That's always an option. :)

@stancl
Copy link
Author

stancl commented Dec 31, 2024

I've realized the source of my confusion here. The fact that libsqlite3 is linked statically means the user's SQLite setup is actually irrelevant, so this is just about packagers choosing to build PHP with or without certain SQLite features. But PHP and the relevant libsqlite should always be in sync then.

Thanks for the clarification.

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

No branches or pull requests

4 participants