Skip to content

Commit

Permalink
implement problem reporting
Browse files Browse the repository at this point in the history
  • Loading branch information
radeusgd committed May 17, 2023
1 parent e1cced3 commit 9b6b641
Show file tree
Hide file tree
Showing 14 changed files with 67 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1328,9 +1328,11 @@ type Column
cast self value_type=Value_Type.Char on_problems=Problem_Behavior.Report_Warning =
Cast_Helpers.check_cast_compatibility self.value_type value_type . if_not_error <|
target_storage_type = Storage.from_value_type value_type on_problems
# TODO problem handling will be handled as part of #5159
new_storage = self.java_column.getStorage.cast target_storage_type
Column.from_storage self.name new_storage
cast_problem_builder = Cast_Helpers.new_java_problem_builder self.name value_type
new_storage = self.java_column.getStorage.cast target_storage_type cast_problem_builder.to_java
problems = cast_problem_builder.get_problems
on_problems.attach_problems_before problems <|
Column.from_storage self.name new_storage

## ALIAS Transform Column

Expand Down
2 changes: 1 addition & 1 deletion distribution/lib/Standard/Table/0.0.0-dev/src/Errors.enso
Original file line number Diff line number Diff line change
Expand Up @@ -564,7 +564,7 @@ type Lossy_Conversion
to_display_text self =
rows_info = case self.affected_rows_count of
Nothing -> ""
Just count -> " in "+count.to_text+" rows"
count -> " in "+count.to_text+" rows"
"When converting the column ["+self.related_column+"], the target type ("+self.target_type.to_display_text+") does not fill all the data, so some information was lost"+rows_info+"."

type Invalid_Value_For_Type
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ import Standard.Base.Errors.Illegal_Argument.Illegal_Argument

import project.Data.Type.Value_Type.Value_Type
import project.Internal.Parse_Values_Helper
from project.Errors import Lossy_Conversion

polyglot java import org.enso.table.data.column.operation.CastProblemBuilder

## PRIVATE
Checks if one type can be cast into another and returns a dataflow error
Expand All @@ -28,3 +31,26 @@ is_a_valid_parse_target target_type =
atom : Meta.Atom ->
Parse_Values_Helper.valid_parse_targets.contains atom.constructor.name
_ -> False

## PRIVATE
type Cast_Problem_Builder
## PRIVATE
Value column_name target_type to_java

## PRIVATE
Returns a vector of all reported problems.
get_problems : Vector
get_problems self =
builder = Vector.new_builder
java_instance = self.to_java

lossy_conversion_rows = java_instance.getLossyConversionRowCount
if lossy_conversion_rows > 0 then
builder.append (Lossy_Conversion.Error self.target_type self.column_name lossy_conversion_rows)

builder.to_vector

## PRIVATE
new_java_problem_builder : Text -> Value_Type -> Cast_Problem_Builder
new_java_problem_builder column_name target_type =
Cast_Problem_Builder.Value column_name target_type CastProblemBuilder.new
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package org.enso.table.data.column.operation;

public class CastProblemBuilder {
private int lossyConversionRowCount = 0;
public void reportLossyConversion() {
lossyConversionRowCount++;
}

public int getLossyConversionRowCount() {
return lossyConversionRowCount;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import org.enso.table.data.column.builder.object.BoolBuilder;
import org.enso.table.data.column.builder.object.Builder;
import org.enso.table.data.column.builder.object.StringBuilder;
import org.enso.table.data.column.builder.string.StringStorageBuilder;
import org.enso.table.data.column.operation.CastProblemBuilder;
import org.enso.table.data.column.operation.map.MapOpStorage;
import org.enso.table.data.column.operation.map.MapOperation;
import org.enso.table.data.column.operation.map.MapOperationProblemBuilder;
Expand Down Expand Up @@ -400,7 +400,7 @@ public BoolStorage slice(List<SliceRange> ranges) {
}

@Override
public Storage<?> cast(StorageType targetType) {
public Storage<?> cast(StorageType targetType, CastProblemBuilder castProblemBuilder) {
return switch (targetType) {
case AnyObjectType any ->
new MixedStorageFacade(this);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import org.enso.table.data.column.builder.object.Builder;
import org.enso.table.data.column.builder.object.DateBuilder;
import org.enso.table.data.column.builder.object.DateTimeBuilder;
import org.enso.table.data.column.operation.CastProblemBuilder;
import org.enso.table.data.column.operation.map.MapOpStorage;
import org.enso.table.data.column.operation.map.UnaryIntegerOp;
import org.enso.table.data.column.operation.map.datetime.DateTimeIsInOp;
Expand Down Expand Up @@ -73,7 +74,7 @@ public Builder createDefaultBuilderOfSameType(int capacity) {
}

@Override
public Storage<?> cast(StorageType targetType) {
public Storage<?> cast(StorageType targetType, CastProblemBuilder castProblemBuilder) {
if (targetType instanceof DateTimeType) {
int n = size();
DateTimeBuilder builder = new DateTimeBuilder(n);
Expand All @@ -88,7 +89,7 @@ public Storage<?> cast(StorageType targetType) {
}
return builder.seal();
} else {
return super.cast(targetType);
return super.cast(targetType, castProblemBuilder);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import org.enso.table.data.column.builder.object.DateBuilder;
import org.enso.table.data.column.builder.object.DateTimeBuilder;
import org.enso.table.data.column.builder.object.TimeOfDayBuilder;
import org.enso.table.data.column.operation.CastProblemBuilder;
import org.enso.table.data.column.operation.map.MapOpStorage;
import org.enso.table.data.column.operation.map.UnaryIntegerOp;
import org.enso.table.data.column.operation.map.datetime.DateTimeIsInOp;
Expand All @@ -14,7 +15,6 @@

import java.time.LocalDate;
import java.time.LocalTime;
import java.time.ZoneId;
import java.time.ZonedDateTime;

public final class DateTimeStorage extends SpecializedStorage<ZonedDateTime> {
Expand Down Expand Up @@ -78,7 +78,7 @@ public Builder createDefaultBuilderOfSameType(int capacity) {
}

@Override
public Storage<?> cast(StorageType targetType) {
public Storage<?> cast(StorageType targetType, CastProblemBuilder castProblemBuilder) {
if (targetType instanceof DateType) {
int n = size();
DateBuilder builder = new DateBuilder(n);
Expand Down Expand Up @@ -106,7 +106,7 @@ public Storage<?> cast(StorageType targetType) {
}
return builder.seal();
} else {
return super.cast(targetType);
return super.cast(targetType, castProblemBuilder);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import org.enso.table.data.column.builder.object.Builder;
import org.enso.table.data.column.builder.object.NumericBuilder;
import org.enso.table.data.column.builder.object.StringBuilder;
import org.enso.table.data.column.operation.CastProblemBuilder;
import org.enso.table.data.column.operation.map.MapOpStorage;
import org.enso.table.data.column.operation.map.MapOperationProblemBuilder;
import org.enso.table.data.column.operation.map.UnaryMapOperation;
Expand Down Expand Up @@ -369,7 +370,7 @@ public Storage<Double> slice(List<SliceRange> ranges) {
}

@Override
public Storage<?> cast(StorageType targetType) {
public Storage<?> cast(StorageType targetType, CastProblemBuilder castProblemBuilder) {
return switch (targetType) {
case AnyObjectType any -> new MixedStorageFacade(this);
case FloatType floatType -> this;
Expand All @@ -384,8 +385,8 @@ public Storage<?> cast(StorageType targetType) {
} else {
double value = getItem(i);
if (value < min || value > max) {
// TODO report warning
builder.appendNulls(1);
castProblemBuilder.reportLossyConversion();
} else {
long converted = (long) value;
builder.appendLong(converted);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import org.enso.table.data.column.builder.object.Builder;
import org.enso.table.data.column.builder.object.NumericBuilder;
import org.enso.table.data.column.builder.object.StringBuilder;
import org.enso.table.data.column.operation.CastProblemBuilder;
import org.enso.table.data.column.operation.map.MapOpStorage;
import org.enso.table.data.column.operation.map.MapOperationProblemBuilder;
import org.enso.table.data.column.operation.map.UnaryMapOperation;
Expand Down Expand Up @@ -458,7 +459,7 @@ public LongStorage slice(List<SliceRange> ranges) {
}

@Override
public Storage<?> cast(StorageType targetType) {
public Storage<?> cast(StorageType targetType, CastProblemBuilder castProblemBuilder) {
return switch (targetType) {
case AnyObjectType any -> new MixedStorageFacade(this);
case IntegerType integerType -> this;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.enso.table.data.column.storage;

import org.enso.table.data.column.builder.object.Builder;
import org.enso.table.data.column.operation.CastProblemBuilder;
import org.enso.table.data.column.operation.map.MapOperationProblemBuilder;
import org.enso.table.data.column.storage.type.AnyObjectType;
import org.enso.table.data.column.storage.type.StorageType;
Expand Down Expand Up @@ -101,7 +102,7 @@ public Storage<Object> slice(List<SliceRange> ranges) {
}

@Override
public Storage<?> cast(StorageType targetType) {
public Storage<?> cast(StorageType targetType, CastProblemBuilder castProblemBuilder) {
return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import java.util.List;

import org.enso.table.data.column.builder.object.StringBuilder;
import org.enso.table.data.column.operation.CastProblemBuilder;
import org.enso.table.data.column.operation.map.MapOpStorage;
import org.enso.table.data.column.operation.map.MapOperationProblemBuilder;
import org.enso.table.data.column.storage.type.AnyObjectType;
Expand Down Expand Up @@ -155,7 +156,7 @@ public SpecializedStorage<T> slice(List<SliceRange> ranges) {
}

@Override
public Storage<?> cast(StorageType targetType) {
public Storage<?> cast(StorageType targetType, CastProblemBuilder castProblemBuilder) {
if (targetType == getType()) {
return this;
} else if (targetType instanceof AnyObjectType) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import org.enso.table.data.column.builder.object.Builder;
import org.enso.table.data.column.builder.object.InferredBuilder;
import org.enso.table.data.column.builder.object.ObjectBuilder;
import org.enso.table.data.column.operation.CastProblemBuilder;
import org.enso.table.data.column.operation.map.MapOperationProblemBuilder;
import org.enso.table.data.column.storage.type.StorageType;
import org.enso.table.data.mask.OrderMask;
Expand Down Expand Up @@ -346,5 +347,5 @@ public Storage<?> duplicateCount() {
return new LongStorage(data);
}

public abstract Storage<?> cast(StorageType targetType);
public abstract Storage<?> cast(StorageType targetType, CastProblemBuilder castProblemBuilder);
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import org.enso.base.Text_Utils;
import org.enso.table.data.column.builder.object.Builder;
import org.enso.table.data.column.builder.object.StringBuilder;
import org.enso.table.data.column.operation.CastProblemBuilder;
import org.enso.table.data.column.operation.map.MapOpStorage;
import org.enso.table.data.column.operation.map.MapOperation;
import org.enso.table.data.column.operation.map.MapOperationProblemBuilder;
Expand Down Expand Up @@ -73,7 +74,7 @@ public Builder createDefaultBuilderOfSameType(int capacity) {
}

@Override
public Storage<?> cast(StorageType targetType) {
public Storage<?> cast(StorageType targetType, CastProblemBuilder castProblemBuilder) {
return switch (targetType) {
case AnyObjectType any -> new MixedStorageFacade(this);
case TextType textType -> adapt(this, textType);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.enso.table_test_helpers;

import org.enso.table.data.column.builder.object.Builder;
import org.enso.table.data.column.operation.CastProblemBuilder;
import org.enso.table.data.column.operation.map.MapOperationProblemBuilder;
import org.enso.table.data.column.storage.Storage;
import org.enso.table.data.column.storage.type.IntegerType;
Expand Down Expand Up @@ -109,7 +110,7 @@ public Storage<Long> slice(List<SliceRange> ranges) {
}

@Override
public Storage<?> cast(StorageType targetType) {
public Storage<?> cast(StorageType targetType, CastProblemBuilder castProblemBuilder) {
return null;
}
}

0 comments on commit 9b6b641

Please sign in to comment.