soumyakanti3578 commented on code in PR #6103:
URL: https://github.com/apache/hive/pull/6103#discussion_r2417849396
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java:
##########
@@ -4918,15 +4920,16 @@ private RelNode genLogicalPlan(QB qb, boolean
outerMostQB,
aliasToRel.put(subqAlias, relNode);
if (qb.getViewToTabSchema().containsKey(subqAlias)) {
- if (relNode instanceof HiveProject) {
- if (this.viewProjectToTableSchema == null) {
- this.viewProjectToTableSchema = new LinkedHashMap<>();
- }
- viewProjectToTableSchema.put((HiveProject) relNode,
qb.getViewToTabSchema().get(subqAlias));
- } else {
- throw new SemanticException("View " + subqAlias + " is
corresponding to "
- + relNode.toString() + ", rather than a HiveProject.");
+ HiveProject project = switch (Objects.requireNonNull(relNode)) {
+ case HiveProject hiveProject -> hiveProject;
Review Comment:
Yeah I noticed that but there's no good solution to it. In the latest commit
it shouldn't complain as the code is:
```
private Optional<HiveProject> extractFirstProject(RelNode rel) {
return switch (rel) {
case HiveProject hiveProject -> Optional.of(hiveProject);
case SingleRel sr -> extractFirstProject(sr.getInput());
case null, default -> Optional.empty();
};
}
```
However, I would have liked to see the cases indented to:
```
private Optional<HiveProject> extractFirstProject(RelNode rel) {
return switch (rel) {
case HiveProject hiveProject -> Optional.of(hiveProject);
case SingleRel sr -> extractFirstProject(sr.getInput());
case null, default -> Optional.empty();
};
}
```
Actually in `checkstyle/checkstyle.xml` we have defined the indentation for
`case`s to be `0`:
```
<module name="Indentation">
<property name="basicOffset" value="2" />
<property name="caseIndent" value="0" />
<property name="throwsIndent" value="4" />
</module>
```
which is fine when you have the older `switch-case` statement, as it doesn't
look too bad:
```
switch(...) {
case a: ...
case b: ...
}
```
But when you have a `switch expression`, like the example shown at the
beginning of this comment (i.e., with a `return`), it makes more sense to treat
it as a `basicOffset` and not a `caseIndent` in my opinion. But we cannot
simply change the value of `caseIndent` as there are older style `switch`
statements in the repo.
Ideally I would like to have a value of `2` for case indents. Also by
default the cases should be indented as seen here:
https://checkstyle.sourceforge.io/checks/misc/indentation.html#caseIndent
Sorry for the long write-up, but we shouldn't see checkstyle warnings in the
latest commit :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
