Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NO-SNOW: Two minor changes that address PR comments and fix code format #679

Merged
merged 14 commits into from
Feb 21, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,7 @@ public ChannelData<T> flush(final String filePath) {
int oldRowCount = 0;
float oldBufferSize = 0F;
long oldRowSequencer = 0;
String oldOffsetToken = null;
String oldEndOffsetToken = null;
String oldStartOffsetToken = null;
Map<String, RowBufferStats> oldColumnEps = null;
Pair<Long, Long> oldMinMaxInsertTimeInMs = null;
Expand All @@ -484,7 +484,7 @@ public ChannelData<T> flush(final String filePath) {
oldRowCount = this.bufferedRowCount;
oldBufferSize = this.bufferSize;
oldRowSequencer = this.channelState.incrementAndGetRowSequencer();
oldOffsetToken = this.channelState.getEndOffsetToken();
oldEndOffsetToken = this.channelState.getEndOffsetToken();
oldStartOffsetToken = this.channelState.getStartOffsetToken();
oldColumnEps = new HashMap<>(this.statsMap);
oldMinMaxInsertTimeInMs =
Expand All @@ -509,7 +509,7 @@ public ChannelData<T> flush(final String filePath) {
data.setRowCount(oldRowCount);
data.setBufferSize(oldBufferSize);
data.setRowSequencer(oldRowSequencer);
data.setOffsetToken(oldOffsetToken);
data.setEndOffsetToken(oldEndOffsetToken);
data.setStartOffsetToken(oldStartOffsetToken);
data.setColumnEps(oldColumnEps);
data.setMinMaxInsertTimeInMs(oldMinMaxInsertTimeInMs);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
*/
class ChannelData<T> {
private Long rowSequencer;
private String offsetToken;
private String endOffsetToken;
private String startOffsetToken;
private T vectors;
private float bufferSize;
Expand Down Expand Up @@ -95,16 +95,16 @@ void setRowSequencer(Long rowSequencer) {
this.rowSequencer = rowSequencer;
}

String getOffsetToken() {
return this.offsetToken;
String getEndOffsetToken() {
return this.endOffsetToken;
}

String getStartOffsetToken() {
return this.startOffsetToken;
}

void setOffsetToken(String offsetToken) {
this.offsetToken = offsetToken;
void setEndOffsetToken(String endOffsetToken) {
this.endOffsetToken = endOffsetToken;
}

void setStartOffsetToken(String startOffsetToken) {
Expand Down Expand Up @@ -164,8 +164,8 @@ public String toString() {
return "ChannelData{"
+ "rowSequencer="
+ rowSequencer
+ ", offsetToken='"
+ offsetToken
+ ", endOffsetToken='"
+ endOffsetToken
+ ", startOffsetToken='"
+ startOffsetToken
+ '\''
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class ChannelMetadata {
private final String channelName;
private final Long clientSequencer;
private final Long rowSequencer;
@Nullable private final String offsetToken;
@Nullable private final String endOffsetToken;
@Nullable private final String startOffsetToken;

static Builder builder() {
Expand Down Expand Up @@ -47,7 +47,6 @@ Builder setOffsetToken(String offsetToken) {
return this;
}


Builder setStartOffsetToken(String startOffsetToken) {
this.startOffsetToken = startOffsetToken;
return this;
Expand All @@ -66,7 +65,7 @@ private ChannelMetadata(Builder builder) {
this.channelName = builder.channelName;
this.clientSequencer = builder.clientSequencer;
this.rowSequencer = builder.rowSequencer;
this.offsetToken = builder.offsetToken;
this.endOffsetToken = builder.offsetToken;
this.startOffsetToken = builder.startOffsetToken;
}

Expand All @@ -88,7 +87,7 @@ Long getRowSequencer() {
@Nullable
@JsonProperty("offset_token")
String getOffsetToken() {
return this.offsetToken;
return this.endOffsetToken;
}

@Nullable
Expand All @@ -100,6 +99,6 @@ String getStartOffsetToken() {
@Nullable
@JsonProperty("end_offset_token")
String getEndOffsetToken() {
return this.offsetToken;
return this.endOffsetToken;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
FileColumnProperties that = (FileColumnProperties) o;
return Objects.equals(columnOrdinal,that.columnOrdinal)
return Objects.equals(columnOrdinal, that.columnOrdinal)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My recent format.sh script suggested me these changes as well.
ahh do you know why it passed in previous PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not really, i don't know why it passed the format gate

&& distinctValues == that.distinctValues
&& nullCount == that.nullCount
&& maxLength == that.maxLength
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ private SerializationResult serializeFromParquetWriteBuffers(
ChannelMetadata.builder()
.setOwningChannelFromContext(data.getChannelContext())
.setRowSequencer(data.getRowSequencer())
.setOffsetToken(data.getOffsetToken())
.setOffsetToken(data.getEndOffsetToken())
.setStartOffsetToken(data.getStartOffsetToken())
.build();
// Add channel metadata to the metadata list
Expand Down Expand Up @@ -153,7 +153,7 @@ private SerializationResult serializeFromJavaObjects(
ChannelMetadata.builder()
.setOwningChannelFromContext(data.getChannelContext())
.setRowSequencer(data.getRowSequencer())
.setOffsetToken(data.getOffsetToken())
.setOffsetToken(data.getEndOffsetToken())
.setStartOffsetToken(data.getStartOffsetToken())
.build();
// Add channel metadata to the metadata list
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,14 @@ public void setupSchema(List<ColumnMetadata> columns) {
addNonNullableFieldName(column.getInternalName());
}
this.statsMap.put(
column.getInternalName(), new RowBufferStats(column.getName(), column.getCollation(), column.getOrdinal()));
column.getInternalName(),
new RowBufferStats(column.getName(), column.getCollation(), column.getOrdinal()));

if (onErrorOption == OpenChannelRequest.OnErrorOption.ABORT
|| onErrorOption == OpenChannelRequest.OnErrorOption.SKIP_BATCH) {
this.tempStatsMap.put(
column.getInternalName(), new RowBufferStats(column.getName(), column.getCollation(), column.getOrdinal()));
column.getInternalName(),
new RowBufferStats(column.getName(), column.getCollation(), column.getOrdinal()));
}

id++;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class RowBufferStats {
RowBufferStats(String columnDisplayName, String collationDefinitionString, int ordinal) {
this.columnDisplayName = columnDisplayName;
this.collationDefinitionString = collationDefinitionString;
this.ordinal= ordinal;
this.ordinal = ordinal;
reset();
}

Expand All @@ -54,7 +54,8 @@ void reset() {

/** Create new statistics for the same column, with all calculated values set to empty */
RowBufferStats forkEmpty() {
return new RowBufferStats(this.getColumnDisplayName(), this.getCollationDefinitionString(), this.getOrdinal());
return new RowBufferStats(
this.getColumnDisplayName(), this.getCollationDefinitionString(), this.getOrdinal());
}

// TODO performance test this vs in place update
Expand All @@ -68,7 +69,8 @@ static RowBufferStats getCombinedStats(RowBufferStats left, RowBufferStats right
left.getCollationDefinitionString(), right.getCollationDefinitionString()));
}
RowBufferStats combined =
new RowBufferStats(left.columnDisplayName, left.getCollationDefinitionString(), left.getOrdinal());
new RowBufferStats(
left.columnDisplayName, left.getCollationDefinitionString(), left.getOrdinal());

if (left.currentMinIntValue != null) {
combined.addIntValue(left.currentMinIntValue);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -797,12 +797,12 @@ public void testBuildAndUpload() throws Exception {
Assert.assertEquals(2, metadataResult.getChannels().size()); // Two channels on the table

Assert.assertEquals("channel1", channelMetadataResult.get(0).getChannelName());
Assert.assertEquals("offset1", channelMetadataResult.get(0).getOffsetToken());
Assert.assertEquals("offset1", channelMetadataResult.get(0).getEndOffsetToken());
Assert.assertEquals(0L, (long) channelMetadataResult.get(0).getRowSequencer());
Assert.assertEquals(0L, (long) channelMetadataResult.get(0).getClientSequencer());

Assert.assertEquals("channel2", channelMetadataResult.get(1).getChannelName());
Assert.assertEquals("offset2", channelMetadataResult.get(1).getOffsetToken());
Assert.assertEquals("offset2", channelMetadataResult.get(1).getEndOffsetToken());
Assert.assertEquals(10L, (long) channelMetadataResult.get(1).getRowSequencer());
Assert.assertEquals(10L, (long) channelMetadataResult.get(1).getClientSequencer());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import net.snowflake.ingest.utils.ErrorCode;
import net.snowflake.ingest.utils.SFException;
import org.apache.commons.codec.binary.Hex;
import org.checkerframework.common.value.qual.IntRange;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
Expand Down Expand Up @@ -105,7 +104,8 @@ static List<ColumnMetadata> createSchema() {
colChar.setLength(11);
colChar.setScale(0);

List<ColumnMetadata> columns = Arrays.asList(
List<ColumnMetadata> columns =
Arrays.asList(
colTinyIntCase, colTinyInt, colSmallInt, colInt, colBigInt, colDecimal, colChar);
for (int i = 0; i < columns.size(); i++) {
columns.get(i).setOrdinal(i + 1);
Expand Down Expand Up @@ -466,7 +466,8 @@ public void testFlush() {
}

private void testFlushHelper(AbstractRowBuffer<?> rowBuffer) {
String offsetToken = "1";
String startOffsetToken = "1";
String endOffsetToken = "2";
Map<String, Object> row1 = new HashMap<>();
row1.put("colTinyInt", (byte) 1);
row1.put("\"colTinyInt\"", (byte) 1);
Expand All @@ -486,16 +487,16 @@ private void testFlushHelper(AbstractRowBuffer<?> rowBuffer) {
row2.put("colChar", "3");

InsertValidationResponse response =
rowBuffer.insertRows(Arrays.asList(row1, row2), offsetToken, offsetToken);
rowBuffer.insertRows(Arrays.asList(row1, row2), startOffsetToken, endOffsetToken);
Assert.assertFalse(response.hasErrors());
float bufferSize = rowBuffer.getSize();

final String filename = "2022/7/13/16/56/testFlushHelper_streaming.bdec";
ChannelData<?> data = rowBuffer.flush(filename);
Assert.assertEquals(2, data.getRowCount());
Assert.assertEquals((Long) 1L, data.getRowSequencer());
Assert.assertEquals(offsetToken, data.getOffsetToken());
Assert.assertEquals(offsetToken, data.getStartOffsetToken());
Assert.assertEquals(startOffsetToken, data.getStartOffsetToken());
Assert.assertEquals(endOffsetToken, data.getEndOffsetToken());
Assert.assertEquals(bufferSize, data.getBufferSize(), 0);

final ParquetChunkData chunkData = (ParquetChunkData) data.getVectors();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -585,7 +585,7 @@ public void testInsertRow() {
Assert.assertEquals(3, data.getRowCount());
Assert.assertEquals((Long) 1L, data.getRowSequencer());
Assert.assertEquals(1, ((ChannelData<ParquetChunkData>) data).getVectors().rows.get(0).size());
Assert.assertEquals("3", data.getOffsetToken());
Assert.assertEquals("3", data.getEndOffsetToken());
Assert.assertEquals("1", data.getStartOffsetToken());
Assert.assertTrue(data.getBufferSize() > 0);
Assert.assertTrue(insertStartTimeInMs <= data.getMinMaxInsertTimeInMs().getFirst());
Expand Down
Loading