Skip to content

Commit adb82de

Browse files
ArnabChatterjee20kArnabChatterjee20k
committed
refactor spatial attribute handling to support NULL values and improve index validation
* fixed long-lat order for geo function * added spatial types as filters * index validation updates * test update for index createion edge cases and filter encoding and decoding
1 parent 7f08617 commit adb82de

4 files changed

Lines changed: 131 additions & 14 deletions

File tree

‎src/Database/Adapter/MariaDB.php‎

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1641,13 +1641,39 @@ protected function getSQLType(string $type, int $size, bool $signed = true, bool
16411641

16421642

16431643
case Database::VAR_POINT:
1644-
return 'POINT' . ($required && !$this->getSupportForSpatialIndexNull() ? ' NOT NULL' : '');
1644+
$type = 'POINT';
1645+
if (!$this->getSupportForSpatialIndexNull()) {
1646+
if ($required) {
1647+
$type .= ' NOT NULL';
1648+
} else {
1649+
$type .= ' NULL';
1650+
}
1651+
}
1652+
return $type;
16451653

16461654
case Database::VAR_LINESTRING:
1647-
return 'LINESTRING' . ($required && !$this->getSupportForSpatialIndexNull() ? ' NOT NULL' : '');
1655+
$type = 'LINESTRING';
1656+
if (!$this->getSupportForSpatialIndexNull()) {
1657+
if ($required) {
1658+
$type .= ' NOT NULL';
1659+
} else {
1660+
$type .= ' NULL';
1661+
}
1662+
}
1663+
return $type;
1664+
16481665

16491666
case Database::VAR_POLYGON:
1650-
return 'POLYGON' . ($required && !$this->getSupportForSpatialIndexNull() ? ' NOT NULL' : '');
1667+
$type = 'POLYGON';
1668+
if (!$this->getSupportForSpatialIndexNull()) {
1669+
if ($required) {
1670+
$type .= ' NOT NULL';
1671+
} else {
1672+
$type .= ' NULL';
1673+
}
1674+
}
1675+
return $type;
1676+
16511677

16521678
default:
16531679
throw new DatabaseException('Unknown type: ' . $type . '. Must be one of ' . Database::VAR_STRING . ', ' . Database::VAR_INTEGER . ', ' . Database::VAR_FLOAT . ', ' . Database::VAR_BOOLEAN . ', ' . Database::VAR_DATETIME . ', ' . Database::VAR_RELATIONSHIP . ', ' . ', ' . Database::VAR_POINT . ', ' . Database::VAR_LINESTRING . ', ' . Database::VAR_POLYGON);

‎src/Database/Database.php‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2263,7 +2263,8 @@ public function updateAttribute(string $collection, string $id, ?string $type =
22632263
$default = null;
22642264
}
22652265

2266-
if ($required === true && in_array($type, Database::SPATIAL_TYPES)) {
2266+
// we need to alter table attribute type to NOT NULL/NULL for change in required
2267+
if (!$this->adapter->getSupportForSpatialIndexNull() && in_array($type, Database::SPATIAL_TYPES)) {
22672268
$altering = true;
22682269
}
22692270

@@ -3323,12 +3324,12 @@ public function createIndex(string $collection, string $id, string $type, array
33233324
if ($type === self::INDEX_SPATIAL) {
33243325
foreach ($attributes as $attr) {
33253326
if (!isset($indexAttributesWithTypes[$attr])) {
3326-
throw new DatabaseException('Attribute "' . $attr . '" not found in collection');
3327+
throw new IndexException('Attribute "' . $attr . '" not found in collection');
33273328
}
33283329

33293330
$attributeType = $indexAttributesWithTypes[$attr];
33303331
if (!in_array($attributeType, [self::VAR_POINT, self::VAR_LINESTRING, self::VAR_POLYGON])) {
3331-
throw new DatabaseException('Spatial index can only be created on spatial attributes (point, linestring, polygon). Attribute "' . $attr . '" is of type "' . $attributeType . '"');
3332+
throw new IndexException('Spatial index can only be created on spatial attributes (point, linestring, polygon). Attribute "' . $attr . '" is of type "' . $attributeType . '"');
33323333
}
33333334
}
33343335

‎src/Database/Validator/Index.php‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -339,9 +339,6 @@ public function getType(): string
339339
public function checkSpatialIndex(Document $index): bool
340340
{
341341
$type = $index->getAttribute('type');
342-
if ($type !== Database::INDEX_SPATIAL) {
343-
return true;
344-
}
345342

346343
if (!$this->spatialIndexSupport) {
347344
$this->message = 'Spatial indexes are not supported';
@@ -351,15 +348,22 @@ public function checkSpatialIndex(Document $index): bool
351348
$attributes = $index->getAttribute('attributes', []);
352349
$orders = $index->getAttribute('orders', []);
353350

351+
if (count($attributes) !== 1) {
352+
$this->message = 'Spatial index can be created on a single spatial attribute';
353+
return false;
354+
}
355+
354356
foreach ($attributes as $attributeName) {
355357
$attribute = $this->attributes[\strtolower($attributeName)] ?? new Document();
356358
$attributeType = $attribute->getAttribute('type', '');
357-
358359
if (!\in_array($attributeType, Database::SPATIAL_TYPES, true)) {
360+
continue;
361+
}
362+
363+
if ($type !== Database::INDEX_SPATIAL) {
359364
$this->message = 'Spatial index can only be created on spatial attributes (point, linestring, polygon). Attribute "' . $attributeName . '" is of type "' . $attributeType . '"';
360365
return false;
361366
}
362-
363367
$required = (bool) $attribute->getAttribute('required', false);
364368
if (!$required && !$this->spatialIndexNullSupport) {
365369
$this->message = 'Spatial indexes do not allow null values. Mark the attribute "' . $attributeName . '" as required or create the index on a column with no null values.';

‎tests/e2e/Adapter/Scopes/SpatialTests.php‎

Lines changed: 89 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use Utopia\Database\Database;
66
use Utopia\Database\Document;
77
use Utopia\Database\Exception;
8+
use Utopia\Database\Exception\Index as IndexException;
89
use Utopia\Database\Exception\Query as QueryException;
910
use Utopia\Database\Exception\Structure as StructureException;
1011
use Utopia\Database\Helpers\ID;
@@ -1469,7 +1470,6 @@ public function testSpatialBulkOperation(): void
14691470
'required' => true,
14701471
'signed' => true,
14711472
'array' => false,
1472-
'filters' => [],
14731473
]),
14741474
new Document([
14751475
'$id' => ID::custom('location'),
@@ -1478,7 +1478,6 @@ public function testSpatialBulkOperation(): void
14781478
'required' => true,
14791479
'signed' => true,
14801480
'array' => false,
1481-
'filters' => [],
14821481
]),
14831482
new Document([
14841483
'$id' => ID::custom('area'),
@@ -1487,7 +1486,6 @@ public function testSpatialBulkOperation(): void
14871486
'required' => false,
14881487
'signed' => true,
14891488
'array' => false,
1490-
'filters' => [],
14911489
])
14921490
];
14931491

@@ -2460,4 +2458,92 @@ public function testSpatialEncodeDecode(): void
24602458
$this->assertEquals($result->getAttribute('line'), null);
24612459
$this->assertEquals($result->getAttribute('poly'), null);
24622460
}
2461+
2462+
public function testSpatialIndexSingleAttributeOnly(): void
2463+
{
2464+
/** @var Database $database */
2465+
$database = static::getDatabase();
2466+
if (!$database->getAdapter()->getSupportForSpatialAttributes()) {
2467+
$this->expectNotToPerformAssertions();
2468+
return;
2469+
}
2470+
2471+
$collectionName = 'spatial_idx_single_attr_' . uniqid();
2472+
try {
2473+
$database->createCollection($collectionName);
2474+
2475+
// Create a spatial attribute
2476+
$database->createAttribute($collectionName, 'loc', Database::VAR_POINT, 0, true);
2477+
$database->createAttribute($collectionName, 'loc2', Database::VAR_POINT, 0, true);
2478+
$database->createAttribute($collectionName, 'title', Database::VAR_STRING, 255, true);
2479+
2480+
// Case 1: Valid spatial index on a single spatial attribute
2481+
$this->assertTrue(
2482+
$database->createIndex($collectionName, 'idx_loc', Database::INDEX_SPATIAL, ['loc'])
2483+
);
2484+
2485+
// Case 2: Fail when trying to create spatial index with multiple attributes
2486+
try {
2487+
$database->createIndex($collectionName, 'idx_multi', Database::INDEX_SPATIAL, ['loc', 'loc2']);
2488+
$this->fail('Expected exception when creating spatial index on multiple attributes');
2489+
} catch (\Throwable $e) {
2490+
$this->assertInstanceOf(IndexException::class, $e);
2491+
}
2492+
2493+
// Case 3: Fail when trying to create non-spatial index on a spatial attribute
2494+
try {
2495+
$database->createIndex($collectionName, 'idx_wrong_type', Database::INDEX_KEY, ['loc']);
2496+
$this->fail('Expected exception when creating non-spatial index on spatial attribute');
2497+
} catch (\Throwable $e) {
2498+
$this->assertInstanceOf(IndexException::class, $e);
2499+
}
2500+
2501+
// Case 4: Fail when trying to mix spatial + non-spatial attributes in a spatial index
2502+
try {
2503+
$database->createIndex($collectionName, 'idx_mix', Database::INDEX_SPATIAL, ['loc', 'title']);
2504+
$this->fail('Expected exception when creating spatial index with mixed attribute types');
2505+
} catch (\Throwable $e) {
2506+
$this->assertInstanceOf(IndexException::class, $e);
2507+
}
2508+
2509+
} finally {
2510+
$database->deleteCollection($collectionName);
2511+
}
2512+
}
2513+
2514+
public function testSpatialIndexRequiredToggling(): void
2515+
{
2516+
/** @var Database $database */
2517+
$database = static::getDatabase();
2518+
if (!$database->getAdapter()->getSupportForSpatialAttributes()) {
2519+
$this->expectNotToPerformAssertions();
2520+
return;
2521+
}
2522+
if ($database->getAdapter()->getSupportForSpatialIndexNull()) {
2523+
$this->expectNotToPerformAssertions();
2524+
return;
2525+
}
2526+
2527+
try {
2528+
$collUpdateNull = 'spatial_idx_toggle';
2529+
$database->createCollection($collUpdateNull);
2530+
2531+
$database->createAttribute($collUpdateNull, 'loc', Database::VAR_POINT, 0, false);
2532+
try {
2533+
$database->createIndex($collUpdateNull, 'idx_loc', Database::INDEX_SPATIAL, ['loc']);
2534+
$this->fail('Expected exception when creating spatial index on NULL-able attribute');
2535+
} catch (\Throwable $e) {
2536+
$this->assertInstanceOf(Exception::class, $e);
2537+
}
2538+
$database->updateAttribute($collUpdateNull, 'loc', required: true);
2539+
$this->assertTrue($database->createIndex($collUpdateNull, 'new index', Database::INDEX_SPATIAL, ['loc']));
2540+
$this->assertTrue($database->deleteIndex($collUpdateNull, 'new index'));
2541+
$database->updateAttribute($collUpdateNull, 'loc', required: false);
2542+
2543+
$database->createDocument($collUpdateNull, new Document(['loc' => null]));
2544+
} finally {
2545+
$database->deleteCollection($collUpdateNull);
2546+
}
2547+
}
2548+
24632549
}

0 commit comments

Comments
 (0)