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.

Sign up for GitHub

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

Jump to bottom

Add the type check when inserting geography value #5460

Merged
merged 3 commits into from
Mar 30, 2023
Merged

Add the type check when inserting geography value #5460

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1bf8e23
Add the type check of when inserting geography value
jievince Mar 30, 2023
d3276c0
fix fallthrough warning
jievince Mar 30, 2023
b8e5ab3
Merge branch 'master' into fix-geo-insert-check
Sophie-Xie Mar 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions src/codec/RowWriterV2.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -656,11 +656,17 @@ WriteResult RowWriterV2::write(ssize_t index, const char* v) {
return write(index, folly::StringPiece(v));
}

WriteResult RowWriterV2::write(ssize_t index, folly::StringPiece v) {
WriteResult RowWriterV2::write(ssize_t index, folly::StringPiece v, bool isWKB) {
auto field = schema_->field(index);
auto offset = headerLen_ + numNullBytes_ + field->offset();
switch (field->type()) {
case PropertyType::GEOGRAPHY: // write wkb
case PropertyType::GEOGRAPHY: {
// If v is a not a WKB string, we need report error.
if (!isWKB) {
return WriteResult::TYPE_MISMATCH;
}
[[fallthrough]];
}
case PropertyType::STRING: {
if (isSet_[index]) {
// The string value has already been set, we need to turn it
Expand Down Expand Up @@ -809,8 +815,10 @@ WriteResult RowWriterV2::write(ssize_t index, const Geography& v) {
folly::to<uint32_t>(geoShape) != folly::to<uint32_t>(v.shape())) {
return WriteResult::TYPE_MISMATCH;
}
// Geography is stored as WKB format.
// WKB is a binary string.
std::string wkb = v.asWKB();
return write(index, folly::StringPiece(wkb));
return write(index, folly::StringPiece(wkb), true);
}

WriteResult RowWriterV2::checkUnsetFields() {
Expand Down
2 changes: 1 addition & 1 deletion src/codec/RowWriterV2.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ class RowWriterV2 {
WriteResult write(ssize_t index, uint64_t v);

WriteResult write(ssize_t index, const std::string& v);
WriteResult write(ssize_t index, folly::StringPiece v);
WriteResult write(ssize_t index, folly::StringPiece v, bool isWKB = false);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you could add a new structure like struct WKB {folly::StringPiece}.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to reuse the code of write(ssize_t index, folly::StringPiece v)

WriteResult write(ssize_t index, const char* v);

WriteResult write(ssize_t index, const Date& v);
Expand Down
8 changes: 8 additions & 0 deletions tests/tck/features/geo/GeoBase.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,14 @@ Feature: Geo base
INSERT VERTEX any_shape(geo) VALUES "103":(ST_GeogFromText("POLYGON((0 1, 1 2, 2 3, 0 1))"));
"""
Then the execution should be successful
# "POINT(3 8)" is a string not a geography literal.
# We must use some geography costructor function to construct a geography literal.
# e.g.`ST_GeogFromText("POINT(3 8)")`
When executing query:
"""
INSERT VERTEX any_shape(geo) VALUES "104":("POINT(3 8)");
"""
Then a ExecutionError should be raised at runtime: Storage Error: The data type does not meet the requirements. Use the correct type of data.
# Only point is allowed to insert to the column geograph(point)
When executing query:
"""
Expand Down