Follow-up: Optimize support recursive delegate unwrapping to find ExtendedParser in parser chains #14497
Conversation
| LOG.warn( | ||
| "Field '{}' not found in class {}. Trying superclass.", | ||
| SQL_PARSER_FIELD, | ||
| clazz.getName()); |
There was a problem hiding this comment.
I also think no need to log in tests.
There was a problem hiding this comment.
@kamcheungting-db: add log to help RCA if something goes wrong
Added logs based on suggestion, just in case.
If you feel they're unnecessary in the test class, we can remove them.
| } | ||
|
|
||
| private static ParserInterface getNextDelegateParser(ParserInterface parser) { | ||
| Logger logger = LoggerFactory.getLogger(ExtendedParser.class); |
There was a problem hiding this comment.
can we not declare this as private static final member of the interface
There was a problem hiding this comment.
Interface fields cannot be private in Java.
Can the code be written like this?
public interface ExtendedParser extends ParserInterface {
Logger LOG = LoggerFactory.getLogger(ExtendedParser.class);
| logger.warn( | ||
| "Failed to scan delegate parser in {}: {}", parser.getClass().getName(), e.toString()); |
There was a problem hiding this comment.
| logger.warn( | |
| "Failed to scan delegate parser in {}: {}", parser.getClass().getName(), e.toString()); | |
| logger.warn("Failed to scan delegate parser in {}", parser.getClass().getName(), e); |
…er in parser chains
…endedParser in parser chains
…endedParser in parser chains
…endedParser in parser chains
…endedParser in parser chains
…endedParser in parser chains
…endedParser in parser chains
majian1998
force-pushed
the
fix-parser-fu-4
branch
from
0925665 to
4c3fd36
Compare
| import org.junit.jupiter.api.Test; | ||
|
|
||
| public class TestExtendedParser { | ||
|
|
…endedParser in parser chains
| clazz = clazz.getSuperclass(); | ||
| } | ||
| } catch (Exception e) { | ||
| // ignore |
There was a problem hiding this comment.
I think we can remove this comment now. This comment was there to suppress "Empty 'catch' block" warning.
| } | ||
| } catch (Exception e) { | ||
| // ignore | ||
| LOG.warn("Failed to scan delegate parser in {}: {}", parser.getClass().getName(), e); |
There was a problem hiding this comment.
We don't need to append : {} for e, right?
There was a problem hiding this comment.
without '{}' will print the exception stacktrace, which actually makes sense here.
…endedParser in parser chains
5 participants
Related PR: #13625, #14483
Some follow-up code optimizations. Thanks to @kamcheungting-db for the suggestions.