-
Notifications
You must be signed in to change notification settings - Fork 1k
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.
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
New data type #12477
New data type #12477
Conversation
RpcUtils.DEFAULT_TIME_FORMAT, | ||
"ms", | ||
BytesUtils.bytesToLong(values[index]), | ||
ZoneId.systemDefault()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should add a zoneId
field?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added.
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/conf/IoTDBDescriptor.java
Outdated
Show resolved
Hide resolved
@@ -95,6 +97,8 @@ public static List<Object> reGenValues(List<TSDataType> types, List<Object> valu | |||
} | |||
break; | |||
case TEXT: | |||
case BLOB: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for Blob, I think they pass String like "X'xaf'", so we need to parse that String.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have not used RestAPI for writing. I need to go through some examples or unit tests to help me with this part later.
@@ -140,6 +142,8 @@ public static InsertTabletStatement constructInsertTabletStatement( | |||
columns[columnIndex] = doubleValues; | |||
break; | |||
case TEXT: | |||
case BLOB: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should parse blob
@@ -147,6 +149,8 @@ public static InsertTabletStatement constructInsertTabletStatement( | |||
columns[columnIndex] = doubleValues; | |||
break; | |||
case TEXT: | |||
case BLOB: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should parse blob
@@ -36,7 +36,8 @@ public enum LiteralType { | |||
DOUBLE, | |||
LONG, | |||
STRING, | |||
NULL | |||
NULL, | |||
BINARY |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why don't you add a switch-case in deserialize
method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added. It seems that Literal doesn't get serialised and deserialised when writing with sql, so I didn't notice.
@@ -43,7 +43,7 @@ public class InsertStatement extends Statement { | |||
private long[] times; | |||
private String[] measurementList; | |||
|
|||
private List<String[]> valuesList; | |||
private List<Object[]> valuesList; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why here need to be changed to Object[], it seems that all the caller is set String[]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I forget why I modified this. But it looks like String[] is correct. I REVERT this modification.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, there are some code in TsFileProcessor I think need to be considered.
Line 592 in b030642
if (dataTypes[i] == TSDataType.TEXT && values[i] != null) { |
@HTHou Thanks for the reminder! I missed some checks on TEXT. I made some changes in commit 228bba7. Please help review them for me~ I think adding an isBinary() method to TSDataType would have been a better choice, but that would have required to modify the TsFile repository. |
+1 |
Hi all, I will close this PR and open a new one for cooperation: #12576 |
Description
Content1 ...
Content2 ...
Content3 ...
This PR has:
for an unfamiliar reader.
for code coverage.
Key changed/added classes (or packages if there are too many classes) in this PR