Copilot commented on code in PR #6762:
URL: https://github.com/apache/hive/pull/6762#discussion_r3976570860
##########
cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java:
##########
@@ -851,7 +851,7 @@ private CommandProcessorResponse
executeDriver(CliSessionState ss, HiveConf conf
String curPrompt = prompt + curDB;
String dbSpaces = spacesForString(curDB);
- while ((line = reader.readLine(curPrompt + "> ")) != null) {
+ while ((line = cli.reader.readLine(curPrompt + "> ")) != null) {
Review Comment:
After switching initialization to `cli.setupLineReader()`, any remaining
unqualified uses of `reader` elsewhere in `executeDriver(...)` would still
refer to `this.reader` (the outer instance) and could reintroduce the same NPE.
Consider assigning `cli.reader` to a local variable (e.g., `LineReader reader =
cli.reader;`) and using that consistently throughout the method to prevent
accidental mixed-instance access.
##########
cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java:
##########
@@ -842,7 +842,7 @@ private CommandProcessorResponse
executeDriver(CliSessionState ss, HiveConf conf
console.printInfo(HiveConf.generateMrDeprecationWarning());
}
- setupLineReader();
+ cli.setupLineReader();
Review Comment:
After switching initialization to `cli.setupLineReader()`, any remaining
unqualified uses of `reader` elsewhere in `executeDriver(...)` would still
refer to `this.reader` (the outer instance) and could reintroduce the same NPE.
Consider assigning `cli.reader` to a local variable (e.g., `LineReader reader =
cli.reader;`) and using that consistently throughout the method to prevent
accidental mixed-instance access.
##########
cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java:
##########
@@ -851,7 +851,7 @@ private CommandProcessorResponse
executeDriver(CliSessionState ss, HiveConf conf
String curPrompt = prompt + curDB;
String dbSpaces = spacesForString(curDB);
- while ((line = reader.readLine(curPrompt + "> ")) != null) {
+ while ((line = cli.reader.readLine(curPrompt + "> ")) != null) {
Review Comment:
Accessing `cli.reader` directly couples `executeDriver(...)` to the field
and makes it easier to accidentally bypass future invariants (e.g., lazy init,
decorators). Prefer routing reads through a method on `CliDriver` (or storing
it in a local `LineReader` variable) to keep the field encapsulated and reduce
future churn if the `reader` implementation changes.
--
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]
