Merge pull request #26540 from nextcloud/bugfix/noid/sqlite-datetime

Cast datetime columns in sqlite before comparing
nextcloud · Dec 29, 2023 · 6a63974 · 6a63974
2 parents 77cec80 + e50cb0a
commit 6a63974
Show file tree

Hide file tree

Showing 4 changed files with 145 additions and 86 deletions.
diff --git a/lib/private/DB/QueryBuilder/ExpressionBuilder/ExpressionBuilder.php b/lib/private/DB/QueryBuilder/ExpressionBuilder/ExpressionBuilder.php
@@ -117,8 +117,8 @@ public function orX(...$x): ICompositeExpression {
 	 * @return string
 	 */
 	public function comparison($x, string $operator, $y, $type = null): string {
-		$x = $this->helper->quoteColumnName($x);
-		$y = $this->helper->quoteColumnName($y);
+		$x = $this->prepareColumn($x, $type);
+		$y = $this->prepareColumn($y, $type);
 		return $this->expressionBuilder->comparison($x, $operator, $y);
 	}
 
@@ -140,8 +140,8 @@ public function comparison($x, string $operator, $y, $type = null): string {
 	 * @return string
 	 */
 	public function eq($x, $y, $type = null): string {
-		$x = $this->helper->quoteColumnName($x);
-		$y = $this->helper->quoteColumnName($y);
+		$x = $this->prepareColumn($x, $type);
+		$y = $this->prepareColumn($y, $type);
 		return $this->expressionBuilder->eq($x, $y);
 	}
 
@@ -162,8 +162,8 @@ public function eq($x, $y, $type = null): string {
 	 * @return string
 	 */
 	public function neq($x, $y, $type = null): string {
-		$x = $this->helper->quoteColumnName($x);
-		$y = $this->helper->quoteColumnName($y);
+		$x = $this->prepareColumn($x, $type);
+		$y = $this->prepareColumn($y, $type);
 		return $this->expressionBuilder->neq($x, $y);
 	}
 
@@ -184,8 +184,8 @@ public function neq($x, $y, $type = null): string {
 	 * @return string
 	 */
 	public function lt($x, $y, $type = null): string {
-		$x = $this->helper->quoteColumnName($x);
-		$y = $this->helper->quoteColumnName($y);
+		$x = $this->prepareColumn($x, $type);
+		$y = $this->prepareColumn($y, $type);
 		return $this->expressionBuilder->lt($x, $y);
 	}
 
@@ -206,8 +206,8 @@ public function lt($x, $y, $type = null): string {
 	 * @return string
 	 */
 	public function lte($x, $y, $type = null): string {
-		$x = $this->helper->quoteColumnName($x);
-		$y = $this->helper->quoteColumnName($y);
+		$x = $this->prepareColumn($x, $type);
+		$y = $this->prepareColumn($y, $type);
 		return $this->expressionBuilder->lte($x, $y);
 	}
 
@@ -228,8 +228,8 @@ public function lte($x, $y, $type = null): string {
 	 * @return string
 	 */
 	public function gt($x, $y, $type = null): string {
-		$x = $this->helper->quoteColumnName($x);
-		$y = $this->helper->quoteColumnName($y);
+		$x = $this->prepareColumn($x, $type);
+		$y = $this->prepareColumn($y, $type);
 		return $this->expressionBuilder->gt($x, $y);
 	}
 
@@ -250,8 +250,8 @@ public function gt($x, $y, $type = null): string {
 	 * @return string
 	 */
 	public function gte($x, $y, $type = null): string {
-		$x = $this->helper->quoteColumnName($x);
-		$y = $this->helper->quoteColumnName($y);
+		$x = $this->prepareColumn($x, $type);
+		$y = $this->prepareColumn($y, $type);
 		return $this->expressionBuilder->gte($x, $y);
 	}
 
@@ -435,4 +435,13 @@ public function castColumn($column, $type): IQueryFunction {
 			$this->helper->quoteColumnName($column)
 		);
 	}
+
+	/**
+	 * @param mixed $column
+	 * @param mixed|null $type
+	 * @return array|IQueryFunction|string
+	 */
+	protected function prepareColumn($column, $type) {
+		return $this->helper->quoteColumnNames($column);
+	}
 }
diff --git a/lib/private/DB/QueryBuilder/ExpressionBuilder/OCIExpressionBuilder.php b/lib/private/DB/QueryBuilder/ExpressionBuilder/OCIExpressionBuilder.php
@@ -39,80 +39,9 @@ class OCIExpressionBuilder extends ExpressionBuilder {
 	protected function prepareColumn($column, $type) {
 		if ($type === IQueryBuilder::PARAM_STR && !is_array($column) && !($column instanceof IParameter) && !($column instanceof ILiteral)) {
 			$column = $this->castColumn($column, $type);
-		} else {
-			$column = $this->helper->quoteColumnNames($column);
 		}
-		return $column;
-	}
-
-	/**
-	 * @inheritdoc
-	 */
-	public function comparison($x, string $operator, $y, $type = null): string {
-		$x = $this->prepareColumn($x, $type);
-		$y = $this->prepareColumn($y, $type);
-
-		return $this->expressionBuilder->comparison($x, $operator, $y);
-	}
-
-	/**
-	 * @inheritdoc
-	 */
-	public function eq($x, $y, $type = null): string {
-		$x = $this->prepareColumn($x, $type);
-		$y = $this->prepareColumn($y, $type);
-
-		return $this->expressionBuilder->eq($x, $y);
-	}
-
-	/**
-	 * @inheritdoc
-	 */
-	public function neq($x, $y, $type = null): string {
-		$x = $this->prepareColumn($x, $type);
-		$y = $this->prepareColumn($y, $type);
-
-		return $this->expressionBuilder->neq($x, $y);
-	}
-
-	/**
-	 * @inheritdoc
-	 */
-	public function lt($x, $y, $type = null): string {
-		$x = $this->prepareColumn($x, $type);
-		$y = $this->prepareColumn($y, $type);
-
-		return $this->expressionBuilder->lt($x, $y);
-	}
-
-	/**
-	 * @inheritdoc
-	 */
-	public function lte($x, $y, $type = null): string {
-		$x = $this->prepareColumn($x, $type);
-		$y = $this->prepareColumn($y, $type);
-
-		return $this->expressionBuilder->lte($x, $y);
-	}
-
-	/**
-	 * @inheritdoc
-	 */
-	public function gt($x, $y, $type = null): string {
-		$x = $this->prepareColumn($x, $type);
-		$y = $this->prepareColumn($y, $type);
-
-		return $this->expressionBuilder->gt($x, $y);
-	}
-
-	/**
-	 * @inheritdoc
-	 */
-	public function gte($x, $y, $type = null): string {
-		$x = $this->prepareColumn($x, $type);
-		$y = $this->prepareColumn($y, $type);
 
-		return $this->expressionBuilder->gte($x, $y);
+		return parent::prepareColumn($column, $type);
 	}
 
 	/**

diff --git a/lib/private/DB/QueryBuilder/ExpressionBuilder/SqliteExpressionBuilder.php b/lib/private/DB/QueryBuilder/ExpressionBuilder/SqliteExpressionBuilder.php
@@ -23,6 +23,12 @@
  */
 namespace OC\DB\QueryBuilder\ExpressionBuilder;
 
+use OC\DB\QueryBuilder\QueryFunction;
+use OCP\DB\QueryBuilder\ILiteral;
+use OCP\DB\QueryBuilder\IParameter;
+use OCP\DB\QueryBuilder\IQueryBuilder;
+use OCP\DB\QueryBuilder\IQueryFunction;
+
 class SqliteExpressionBuilder extends ExpressionBuilder {
 	/**
 	 * @inheritdoc
@@ -34,4 +40,33 @@ public function like($x, $y, $type = null): string {
 	public function iLike($x, $y, $type = null): string {
 		return $this->like($this->functionBuilder->lower($x), $this->functionBuilder->lower($y), $type);
 	}
+
+	/**
+	 * @param mixed $column
+	 * @param mixed|null $type
+	 * @return array|IQueryFunction|string
+	 */
+	protected function prepareColumn($column, $type) {
+		if ($type === IQueryBuilder::PARAM_DATE && !is_array($column) && !($column instanceof IParameter) && !($column instanceof ILiteral)) {
+			return $this->castColumn($column, $type);
+		}
+
+		return parent::prepareColumn($column, $type);
+	}
+
+	/**
+	 * Returns a IQueryFunction that casts the column to the given type
+	 *
+	 * @param string $column
+	 * @param mixed $type One of IQueryBuilder::PARAM_*
+	 * @return IQueryFunction
+	 */
+	public function castColumn($column, $type): IQueryFunction {
+		if ($type === IQueryBuilder::PARAM_DATE) {
+			$column = $this->helper->quoteColumnName($column);
+			return new QueryFunction('DATETIME(' . $column . ')');
+		}
+
+		return parent::castColumn($column, $type);
+	}
 }
diff --git a/tests/lib/DB/QueryBuilder/ExpressionBuilderDBTest.php b/tests/lib/DB/QueryBuilder/ExpressionBuilderDBTest.php
@@ -21,8 +21,12 @@
 
 namespace Test\DB\QueryBuilder;
 
+use Doctrine\DBAL\Schema\SchemaException;
+use Doctrine\DBAL\Types\Types;
 use OC\DB\QueryBuilder\Literal;
 use OCP\DB\QueryBuilder\IQueryBuilder;
+use OCP\IConfig;
+use OCP\Server;
 use Test\TestCase;
 
 /**
@@ -31,11 +35,13 @@
 class ExpressionBuilderDBTest extends TestCase {
 	/** @var \Doctrine\DBAL\Connection|\OCP\IDBConnection */
 	protected $connection;
+	protected $schemaSetup = false;
 
 	protected function setUp(): void {
 		parent::setUp();
 
 		$this->connection = \OC::$server->getDatabaseConnection();
+		$this->prepareTestingTable();
 	}
 
 	public function likeProvider() {
@@ -150,6 +156,59 @@ public function testLongText(): void {
 		self::assertEquals('myvalue', $entries[0]['configvalue']);
 	}
 
+	public function testDateTimeEquals() {
+		$dateTime = new \DateTime('2023-01-01');
+		$insert = $this->connection->getQueryBuilder();
+		$insert->insert('testing')
+			->values(['datetime' => $insert->createNamedParameter($dateTime, IQueryBuilder::PARAM_DATE)])
+			->executeStatement();
+
+		$query = $this->connection->getQueryBuilder();
+		$result = $query->select('*')
+			->from('testing')
+			->where($query->expr()->eq('datetime', $query->createNamedParameter($dateTime, IQueryBuilder::PARAM_DATE)))
+			->executeQuery();
+		$entries = $result->fetchAll();
+		$result->closeCursor();
+		self::assertCount(1, $entries);
+	}
+
+	public function testDateTimeLess() {
+		$dateTime = new \DateTime('2022-01-01');
+		$dateTimeCompare = new \DateTime('2022-01-02');
+		$insert = $this->connection->getQueryBuilder();
+		$insert->insert('testing')
+			->values(['datetime' => $insert->createNamedParameter($dateTime, IQueryBuilder::PARAM_DATE)])
+			->executeStatement();
+
+		$query = $this->connection->getQueryBuilder();
+		$result = $query->select('*')
+			->from('testing')
+			->where($query->expr()->lt('datetime', $query->createNamedParameter($dateTimeCompare, IQueryBuilder::PARAM_DATE)))
+			->executeQuery();
+		$entries = $result->fetchAll();
+		$result->closeCursor();
+		self::assertCount(1, $entries);
+	}
+
+	public function testDateTimeGreater() {
+		$dateTime = new \DateTime('2023-01-02');
+		$dateTimeCompare = new \DateTime('2023-01-01');
+		$insert = $this->connection->getQueryBuilder();
+		$insert->insert('testing')
+			->values(['datetime' => $insert->createNamedParameter($dateTime, IQueryBuilder::PARAM_DATE)])
+			->executeStatement();
+
+		$query = $this->connection->getQueryBuilder();
+		$result = $query->select('*')
+			->from('testing')
+			->where($query->expr()->gt('datetime', $query->createNamedParameter($dateTimeCompare, IQueryBuilder::PARAM_DATE)))
+			->executeQuery();
+		$entries = $result->fetchAll();
+		$result->closeCursor();
+		self::assertCount(1, $entries);
+	}
+
 	protected function createConfig($appId, $key, $value) {
 		$query = $this->connection->getQueryBuilder();
 		$query->insert('appconfig')
@@ -160,4 +219,31 @@ protected function createConfig($appId, $key, $value) {
 			])
 			->execute();
 	}
+
+	protected function prepareTestingTable(): void {
+		if ($this->schemaSetup) {
+			$this->connection->getQueryBuilder()->delete('testing')->executeStatement();
+		}
+
+		$prefix = Server::get(IConfig::class)->getSystemValueString('dbtableprefix', 'oc_');
+		$schema = $this->connection->createSchema();
+		try {
+			$schema->getTable($prefix . 'testing');
+			$this->connection->getQueryBuilder()->delete('testing')->executeStatement();
+		} catch (SchemaException $e) {
+			$this->schemaSetup = true;
+			$table = $schema->createTable($prefix . 'testing');
+			$table->addColumn('id', Types::BIGINT, [
+				'autoincrement' => true,
+				'notnull' => true,
+			]);
+
+			$table->addColumn('datetime', Types::DATETIME_MUTABLE, [
+				'notnull' => false,
+			]);
+
+			$table->setPrimaryKey(['id']);
+			$this->connection->migrateToSchema($schema);
+		}
+	}
 }