flink-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dawid Wysakowicz (Jira)" <j...@apache.org>
Subject [jira] [Commented] (FLINK-14296) SqlNodes in the parser module should use an Optional for optional parameters
Date Wed, 09 Oct 2019 09:37:00 GMT

    [ https://issues.apache.org/jira/browse/FLINK-14296?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16947486#comment-16947486
] 

Dawid Wysakowicz commented on FLINK-14296:
------------------------------------------

Sorry I made you wait for a response.

I would really suggest first checking and agreeing on what is the actual contract of those
properties. The ctor of e.g. SqlCreateTable ensures that only tableName and columnList are
not null. The remaining fields are nullable. Moreover even in the method {{SqlCreateTable#validate}}
we are checking {{columnList}} against null, so I really doubt your claim that this is always
an empty {{SqlNodeList}} stands.

The fact that we do not support the primary keys and unique keys is not an argument in here.
If we do not support them why not remove them from SqlCreateTable? (I am not suggesting to
actually remove it,  just showing that the argument is invalid).

Just to summarize, I'm fine with either returning an {{Optional}} or an object and not checking
against null. This has to be consistent and ensured though.

> SqlNodes in the parser module should use an Optional for optional parameters
> ----------------------------------------------------------------------------
>
>                 Key: FLINK-14296
>                 URL: https://issues.apache.org/jira/browse/FLINK-14296
>             Project: Flink
>          Issue Type: Improvement
>          Components: Table SQL / API
>            Reporter: Dawid Wysakowicz
>            Priority: Major
>              Labels: pull-request-available
>          Time Spent: 10m
>  Remaining Estimate: 0h
>
> I want to suggest using Optional for optional parameters in classes such as SqlCreateTable/SqlCreateView/SqlTableColumn
etc.
> Right now we must check against null at different locations e.g.:
> {code}
> SqlNodeList partitionKey = sqlCreateTable.getPartitionKeyList();
> if (partitionKey != null) {
> 	partitionKeys = partitionKey
> 		.getList()
> 		.stream()
> 		.map(p -> ((SqlIdentifier) p).getSimple())
> 		.collect(Collectors.toList());
> }
> {code}



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Mime
View raw message