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

DB binding is wailing or broken with some PG operators #731

Open
DeBug88 opened this issue Feb 12, 2025 · 3 comments
Open

DB binding is wailing or broken with some PG operators #731

DeBug88 opened this issue Feb 12, 2025 · 3 comments

Comments

@DeBug88
Copy link

DeBug88 commented Feb 12, 2025

PHP 8.3
Clockwork 5.3.*
DB: PostgreSQL 14.3 (any with json operations)

Prepare:

CREATE TABLE temp (
    id INT PRIMARY KEY,
    json jsonb NOT NULL DEFAULT '{}'
);

INSERT INTO temp(id, json) VALUES (1, '{"a": 5}'), (2, '{"a": 1, "b": 3}');

Case 1

SELECT id, json ?? 'a' AS has_a
FROM temp t
WHERE id = ?

Gives us wrong binding results
Image
Image

Case 2

SELECT id, json ?? 'a' AS has_a
FROM temp t
WHERE id = :id

Cause Fatal error
"message":"Undefined array key 1", "file":"/app/vendor/itsgoingd/clockwork/Clockwork/DataSource/EloquentDataSource.php:274"

Issue is in following preg pattern
$query = preg_replace_callback('/\?/', function ($matches) use ($bindings, $connection, &$index) {

it's not working with json operations like in PG: https://www.postgresql.org/docs/current/functions-json.html#FUNCTIONS-JSON-PROCESSING
see Table 9.46. Additional jsonb Operators

jsonb ? text → boolean
jsonb @? jsonpath → boolean

See https://jdbc.postgresql.org/documentation/query/#using-the-statement-or-preparedstatement-interface

@DeBug88
Copy link
Author

DeBug88 commented Feb 12, 2025

I build the followin binding using laravel core functionality to bind values when debug queries

$bindings = $this->databaseManager->connection($connection)->prepareBindings($bindings);
$query = $this->databaseManager->connection($connection)->getQueryGrammar()->substituteBindingsIntoRawSql($query, $bindings);
foreach ($bindings as $key => $value) {
    // index bindings already done in substitue binding before
    if (str_starts_with($key, ':')) {
        $query = str_replace($key, $value, $query);
    }
}

but i'm unsure about everything inside closure

$query = preg_replace_callback('/\?/', function ($matches) use ($bindings, $connection, &$index) {
	$binding = $this->quoteBinding($bindings[$index++], $connection);

	// convert binary bindings to hexadecimal representation
	if (! preg_match('//u', (string) $binding)) $binding = '0x' . bin2hex($binding);

	// escape backslashes in the binding (preg_replace requires to do so)
	return (string) $binding;
}, $query, count($bindings));

@itsgoingd
Copy link
Owner

Hey, thanks for the report. Another issue in our implementation is an improper handling of ? characters inside 'quoted strings'. I think we can fix our regex to fix these cases, or we could just use Laravel's implementation.

@itsgoingd
Copy link
Owner

I've improved the query generation code in master (29095a5), can you please try if this solves your issue?

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

No branches or pull requests

2 participants