Skip to content

[IOTDB-1515]Fix binary convertion in LastQueryExecutor#3605

Closed
ericpai wants to merge 1 commit intoapache:masterfrom
ericpai:bugfix/last-query-binary
Closed

[IOTDB-1515]Fix binary convertion in LastQueryExecutor#3605
ericpai wants to merge 1 commit intoapache:masterfrom
ericpai:bugfix/last-query-binary

Conversation

@ericpai
Copy link
Copy Markdown
Member

@ericpai ericpai commented Jul 20, 2021

Please see JIRA https://issues.apache.org/jira/browse/IOTDB-1515 .

Here are another two questions. Any guidance or discussion is welcomed :).

  1. What is the Binary type designed for? Any bytes or only valid UTF-8 strings?
  2. If any bytes is allowed, the implementation of AbstractIoTDBJDBCResultSet.getBytes is needed. As we cannot restore orignal binary data from a malformed UTF-8 string.

If this PR is accepted. I think it should be cherry-picked to release branches.

@HTHou
Copy link
Copy Markdown
Contributor

HTHou commented Jul 20, 2021

Hi, currently we can only insert any bytes with Binary by insertTablet interface, right? InsertRecord interface only support insert String record. If we consider to support any bytes, some modification of insertRecord is needed I think…

@coveralls
Copy link
Copy Markdown

coveralls commented Jul 20, 2021

Coverage Status

Coverage decreased (-0.01%) to 68.151% when pulling 01cc43d on ericpai:bugfix/last-query-binary into 79e50d6 on apache:master.

@ericpai ericpai force-pushed the bugfix/last-query-binary branch from 575c95b to 01cc43d Compare July 21, 2021 00:28
@ericpai
Copy link
Copy Markdown
Member Author

ericpai commented Jul 21, 2021

some modification of insertRecord is needed I think…

@HTHou Yes, InsertRecord treats the input TEXT type data as a String, not byte[]. But user will be confused if he uses InsertTablet API to insert Binary objects and gets unexcepted ones.

Maybe we can indicate that only valid UTF-8 String supported in TEXT type in InsertRecord(), and it will support any bytes in the future. It's backward compatible I think. But if we want to support any bytes, the JDBC interface should implement getBytes(). I could help to do this.

However this PR is the superset of UTF-8 string support(I can add another test case for valid utf8 bytes). Do you think it should be merged?

@qiaojialin
Copy link
Copy Markdown
Member

Hi, the data type of Binary is TEXT, so it's before only store UTF-8 String.

If we want to store bytes, we could import another data type called BINARY.

This could be discussed in the mail list :)

@ericpai
Copy link
Copy Markdown
Member Author

ericpai commented Jul 21, 2021

Hi, the data type of Binary is TEXT, so it's before only store UTF-8 String.

If we want to store bytes, we could import another data type called BINARY.

This could be discussed in the mail list :)

It makes sense! Currently I will use base64 encoding as a workaround solution to store arbitrary bytes. This PR and JIRA will be closed.

@ericpai ericpai closed this Jul 21, 2021
@jixuan1989
Copy link
Copy Markdown
Member

@ericpai , we indeed need a new binary (better to be called as Blob) data type, which stores bytes and has no statistics like sum etc (first value and last value are also not needed by default)..

Then, current binary can be renamed as Clob or Text directly (but we need to consider compatibility)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants