[CORE] Merge SubstraitUtil classes#10587
Conversation
|
Run Gluten Clickhouse CI on x86 |
| if (obj == null) return null | ||
| var msg: Message = null | ||
| obj match { | ||
| case Some(s: String) => msg = StringValue.newBuilder.setValue(s).build |
There was a problem hiding this comment.
Sorry, I'm new to Scala, I was having trouble getting this code to compile and that was something I found that worked. Spoke with someone more knowledgeable than me and they pointed out what I was doing wrong. Fixed.
kevinwilfong
force-pushed
the
substrait_utils
branch
from
bee65a8 to
5ac396d
Compare
|
Run Gluten Clickhouse CI on x86 |
|
|
||
| def convertJavaObjectToAny(obj: AnyRef): Any = { | ||
| if (obj == null) return null | ||
| var msg: Message = null |
There was a problem hiding this comment.
suggestion:
val msg = obj match {
***
}
There was a problem hiding this comment.
I had to leave in the : Message otherwise I got a compiler error I think because the types in the different cases are different, but otherwise made the change.
| // TODO: generate the message according to the object type | ||
| msg = StringValue.newBuilder.setValue(obj.toString).build | ||
| } | ||
| if (msg == null) return null |
There was a problem hiding this comment.
You're right, I copied that from the original code, but the contract of the build functions is that they don't return null so it's safe to remove it.
kevinwilfong
force-pushed
the
substrait_utils
branch
from
5ac396d to
5eea690
Compare
|
Run Gluten Clickhouse CI on x86 |
|
ClickHouse CI failed with org.apache.hadoop.ipc.RemoteException(java.io.IOException): File /testFile.txt.COPYING could only be replicated to 0 nodes instead of minReplication (=1). There are 0 datanode(s) running and no node(s) are excluded in this operation. I assume this is transient |
|
Run Gluten Clickhouse CI on x86 |
|
Thanks @zml1206 ! |
3 participants
What changes are proposed in this pull request?
There are two classes called SubstraitUtil, one in gluten/utils and one in gluten/substrait/utils. The latter has one method
used in one location. This PR refactors them into a single class.
I ran into this when I tried to import both in the same file.
How was this patch tested?
Existing unit tests, this is just a refactor.