Handle dates specifically

[Yopai]

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)

[MasterOdin]

Could you provide an example migration where you're hitting an issue?

[Yopai]

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.

[MasterOdin]

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.

[Yopai]

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.

[dereuromark]

Did that PR ever make it?
Afaik we fixed up the date topic in the meantime, no?