[KYUUBI #1579] Implement basic ability of executing statement in Flink engine #1603
Conversation
|
|
||
| <dependency> | ||
| <groupId>org.apache.flink</groupId> | ||
| <artifactId>flink-test-utils_2.12</artifactId> |
There was a problem hiding this comment.
Kyuubi uses log4j12, but this has transitive deps log4j2. We should exclude other log4j2 deps except to log4j2-core, and bridge it to sfl4j
There was a problem hiding this comment.
nit: 2.12 => ${scala_binary_version}
| import java.util.stream.IntStream | ||
|
|
||
| import scala.collection.JavaConversions._ | ||
| import scala.collection.JavaConverters.collectionAsScalaIterableConverter |
There was a problem hiding this comment.
JavaConverters is deprecated, use import scala.collection.JavaConversions._ and [util.List object].asScala
| Thread.sleep(50) // slow the processing down | ||
|
|
||
| val result = executor.snapshotResult(sessionId, resultID, 2) | ||
| if (result.getType eq TypedResult.ResultType.PAYLOAD) { |
There was a problem hiding this comment.
can we use
result.getType match {
case TypedResult.ResultType.PAYLOAD =>
case TypedResult.ResultType.EOS =>
}
here?
|
|
||
| private var resultDescriptor: ResultDescriptor = _ | ||
|
|
||
| private var columnInfos: List[ColumnInfo] = _ |
There was a problem hiding this comment.
List[ColumnInfo] => util.List[ColumnInfo]
|
|
||
| foo(page) | ||
| }) | ||
| } else if (result.getType eq TypedResult.ResultType.EOS) loop = false |
There was a problem hiding this comment.
Ideally, we should return a iterator and lazy fetch the next snapshot (or periodically update the snapshot), but it's ok for now, as we just tend to support the simplest query select 1 in this initial PR.
pan3793
changed the title
Codecov Report
@@ Coverage Diff @@
## master #1603 +/- ##
============================================
- Coverage 58.96% 58.23% -0.74%
+ Complexity 196 163 -33
============================================
Files 255 257 +2
Lines 12663 12797 +134
Branches 1601 1610 +9
============================================
- Hits 7467 7452 -15
- Misses 4560 4719 +159
+ Partials 636 626 -10
Continue to review full report at Codecov.
|
pan3793
changed the title
|
Thanks, merging to master for v1.5 |
Why are the changes needed?
How was this patch tested?
Add some test cases that check the changes thoroughly including negative and positive cases if possible
Add screenshots for manual tests if appropriate
Run test locally before make a pull request