aihuaxu commented on code in PR #12138:
URL: https://github.com/apache/iceberg/pull/12138#discussion_r1937727913
##########
core/src/main/java/org/apache/iceberg/variants/VariantObject.java:
##########
@@ -38,4 +38,23 @@ default Variants.PhysicalType type() {
default VariantObject asObject() {
return this;
}
+
+ static String asString(VariantObject object) {
Review Comment:
Same here. Probably add as regular class method rather than static funciton.
##########
core/src/main/java/org/apache/iceberg/variants/PrimitiveWrapper.java:
##########
@@ -210,4 +210,9 @@ public int writeTo(ByteBuffer outBuffer, int offset) {
throw new UnsupportedOperationException("Unsupported primitive type: " +
type());
}
+
+ @Override
+ public String toString() {
Review Comment:
Can we add basic test coverage for the changes?
##########
core/src/main/java/org/apache/iceberg/variants/VariantMetadata.java:
##########
@@ -34,4 +34,20 @@ public interface VariantMetadata extends Variants.Serialized
{
/** Returns the size of the metadata dictionary. */
int dictionarySize();
+
+ static String asString(VariantMetadata metadata) {
+ StringBuilder builder = new StringBuilder();
Review Comment:
Maybe just add as regular method rather than static function.
That also will be consistent with other conversion like as asStructType() in
Type.java.
##########
core/src/main/java/org/apache/iceberg/variants/VariantPrimitive.java:
##########
@@ -26,4 +31,23 @@ public interface VariantPrimitive<T> extends VariantValue {
default VariantPrimitive<?> asPrimitive() {
return this;
}
+
+ private String valueAsString() {
+ switch (type()) {
+ case DATE:
+ return DateTimeUtil.daysToIsoDate((Integer) get());
+ case TIMESTAMPTZ:
+ return DateTimeUtil.microsToIsoTimestamptz((Long) get());
+ case TIMESTAMPNTZ:
+ return DateTimeUtil.microsToIsoTimestamp((Long) get());
+ case BINARY:
+ return
BaseEncoding.base16().encode(ByteBuffers.toByteArray((ByteBuffer) get()));
+ default:
+ return String.valueOf(get());
+ }
+ }
+
+ static String asString(VariantPrimitive<?> primitive) {
Review Comment:
Same here. Probably add as regular class method rather than static funciton.
##########
core/src/main/java/org/apache/iceberg/variants/VariantMetadata.java:
##########
@@ -34,4 +34,20 @@ public interface VariantMetadata extends Variants.Serialized
{
/** Returns the size of the metadata dictionary. */
int dictionarySize();
+
+ static String asString(VariantMetadata metadata) {
+ StringBuilder builder = new StringBuilder();
+
+ builder.append("VariantMetadata(dict={");
+ for (int i = 0; i < metadata.dictionarySize(); i += 1) {
+ if (i > 0) {
+ builder.append(", ");
+ }
+
+ builder.append(i).append(" => ").append(metadata.get(i));
Review Comment:
+1. Basically we are creating an ID => object key string mapping. Seems fine
to use ":" to be consistent.
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
