Handle dates specifically
Hi,
=== Context Phinx :) for this work.
I'm writing an Adapter for PDO-OCI.
I'm getting blocked by some things that should be fixed in.
=== Observed behaviour The PDOAdapter generates the query with date formatted as string. Oracle generates an ORA-01843 error (invalid month). Setting NLS_DATE_FORMAT has no effect (it may be related to the Oracle version)
== Proposed behaviour As done for boolean, use a "castDate" method. Which would, for others, use the string as is. And could be overriden in OCIAdapter to encapsulate the string in a "TO_DATE".
Another way to do it, probably better, would be used parametrized queries.
(For now, I have overriden the migrated method in OCI-Adapter, I will push a PR when I'll find some time)
Could you provide an example migration where you're hitting an issue?
No need for a specific migration. The insert done by 'migrated' use dates as string, that don't work on (at least my instance of) Oracle.
Oh I see, you're talking about the PdoAdapter::migrated function. You're envisioning modifying the insert to be something like:
$sql = sprintf(
"INSERT INTO %s (%s, %s, %s, %s, %s) VALUES ('%s', '%s', '%s', '%s', %s);",
$this->quoteTableName($this->getSchemaTableName()),
$this->quoteColumnName('version'),
$this->quoteColumnName('migration_name'),
$this->quoteColumnName('start_time'),
$this->quoteColumnName('end_time'),
$this->quoteColumnName('breakpoint'),
$migration->getVersion(),
substr($migration->getName(), 0, 100),
$this->castToTimestamp($startTime),
$this->castToTimestamp($endTime),
$this->castToBool(false)
);
I think this makes sense.
Yes, that's it. My PR is quite ready, I will push it today.
Generally speaking, I have another problem : Oracle is case sensitive on column names, and so, doing "SELECT *" returns record with key VERSION, and not version.
Si I have multiple ideas, I wanted to have your opininon on each.
- instead of creating SQL in functional methods (like migrated), I think it would be cleanest to handle a Query builder for DQL (SELECT) and DML (INSRET, UPDATE, DELETE - I need to check if they already exist), queries, allowing a syntax like : ->select([fields]) ->from($table) ->where ...
It could be used in migrations, too (because sometimes you need to SELECT things in migrations) It could handle specific cases, like the offset/count clauses, that have totally different syntaxes
- the return of a fetch should handle case transformations, to alllow a "SELECT version" call return a key named "version", and not VERSION
I will push Draft PRs to explain what I mean.
Did that PR ever make it? Afaik we fixed up the date topic in the meantime, no?