Skip to content

[Table Model] Schema Validation Interface#12707

Merged
JackieTien97 merged 4 commits intoapache:ty/TableModelGrammarfrom
MarcosZyk:schema-fetcher-interface
Jun 12, 2024
Merged

[Table Model] Schema Validation Interface#12707
JackieTien97 merged 4 commits intoapache:ty/TableModelGrammarfrom
MarcosZyk:schema-fetcher-interface

Conversation

@MarcosZyk
Copy link
Contributor

@MarcosZyk MarcosZyk commented Jun 11, 2024

Description

Setup schema validation interfaces, including table schema validation and device schema validation.

* This class acts as a request for device schema validation and defines the necessary information
* interfaces.
*/
public interface ITableDeviceSchemaValidation {
Copy link
Contributor

Choose a reason for hiding this comment

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

add java doc for each method and examples

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added.


List<String> getAttributeColumnNameList();

List<List<String>> getAttributeValue();
Copy link
Contributor

Choose a reason for hiding this comment

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

column-based or row-based?

Copy link
Contributor

Choose a reason for hiding this comment

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

List<Object[]> will be better? If we extend attribute columns to allow other types.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Row based.


String getTableName();

List<String[]> getDeviceIdList();
Copy link
Contributor

Choose a reason for hiding this comment

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

column-based or row-based?

Copy link
Contributor

Choose a reason for hiding this comment

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

List<Object[]> will be better? If we extend idcolumns to allow other types.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Row-based. Already update the interface.

* <p>When the input dataType or category of one column is null, the column cannot be auto
* created.
*/
TableSchema validateTableHeaderSchema(
Copy link
Contributor

Choose a reason for hiding this comment

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

What if auto create is false? So I think should return Optional, if we cannot auto craete this table we just return Optinal.empty()

Copy link
Contributor

Choose a reason for hiding this comment

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

what if schema validation failed? dataType miss match? throw what kind of exception?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Throw SemanticException

* <p>When device attribute is missing or different from that stored in IoTDB, the attribute will
* be auto upsert.
*/
void validateDeviceSchema(ITableDeviceSchemaValidation schemaValidation, MPPQueryContext context);
Copy link
Contributor

Choose a reason for hiding this comment

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

think about partial insert, how to return info to caller about:

  1. auto create failed(because auto_create is false or other reasons like exceed device limit)
  2. auto create succeed or this device already exists before.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

First, We haven't define what is "partial insert" in table model. Device level or series level? Second, to support partial insert, another interface "markFailed(int index)" shall be added to the ITableDeviceSchemaValidation and will be recalled during validation process.

For "auto create succeed or this device already exists before", no exception or info should be return in current stage. Id column data type may be should be returned if we support device id values with different data type.

Copy link
Contributor

Choose a reason for hiding this comment

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

After confirmation with @jt2594838 and @qiaojialin , auto create won't affect device creation, so the creation for device won't failed because of auto_create is false, but there may be other reasons.

If your current implementation will throw some specific exception if any device creation failed, it's ok but you need add that exception in java doc and make your interface more clear.

Copy link
Contributor

@jt2594838 jt2594838 left a comment

Choose a reason for hiding this comment

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

Please resolve the problems @JackieTien97 has mentioned.

@JackieTien97 JackieTien97 merged commit 50d8708 into apache:ty/TableModelGrammar Jun 12, 2024
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.

3 participants