Skip to content

Correcting the count, exits and empty for empty collections. #2208

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

Merged
merged 3 commits into from
Mar 5, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import au.csiro.pathling.fhirpath.FhirPathType;
import au.csiro.pathling.fhirpath.column.ColumnRepresentation;
import au.csiro.pathling.fhirpath.column.EmptyRepresentation;
import au.csiro.pathling.fhirpath.column.DefaultRepresentation;
import au.csiro.pathling.fhirpath.definition.NodeDefinition;
import au.csiro.pathling.fhirpath.operator.Comparable;
import jakarta.annotation.Nonnull;
Expand All @@ -16,7 +16,7 @@
public class EmptyCollection extends Collection {

private static final EmptyCollection INSTANCE = new EmptyCollection(
EmptyRepresentation.getInstance(), Optional.empty(), Optional.empty(), Optional.empty(),
DefaultRepresentation.empty(), Optional.empty(), Optional.empty(), Optional.empty(),
Optional.empty());

protected EmptyCollection(@Nonnull final ColumnRepresentation column,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package au.csiro.pathling.fhirpath.collection.mixed;

import au.csiro.pathling.fhirpath.collection.Collection;
import au.csiro.pathling.fhirpath.column.EmptyRepresentation;
import au.csiro.pathling.fhirpath.column.DefaultRepresentation;
import au.csiro.pathling.fhirpath.definition.ChoiceDefinition;
import jakarta.annotation.Nonnull;
import java.util.Optional;
Expand All @@ -16,7 +16,7 @@
public abstract class MixedCollection extends Collection {

protected MixedCollection() {
super(EmptyRepresentation.getInstance(), Optional.empty(), Optional.empty(), Optional.empty(),
super(DefaultRepresentation.empty(), Optional.empty(), Optional.empty(), Optional.empty(),
Optional.empty());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
* @author John Grimes
*/
public abstract class ColumnRepresentation {

/**
* Create a new {@link ColumnRepresentation} from the result of a function that takes two
* {@link Column} operands and returns a single {@link Column} result.
Expand Down Expand Up @@ -414,7 +414,7 @@ public ColumnRepresentation countDistinct() {
* @return A new {@link ColumnRepresentation} that is the result of the check
*/
@Nonnull
public ColumnRepresentation empty() {
public ColumnRepresentation isEmpty() {
return vectorize(
c -> when(c.isNotNull(), size(c).equalTo(0)).otherwise(true),
Column::isNull);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import lombok.Getter;
import lombok.ToString;
import org.apache.spark.sql.Column;
import org.apache.spark.sql.functions;
import org.hl7.fhir.r4.model.Enumerations.FHIRDefinedType;

/**
Expand All @@ -44,7 +45,23 @@
@AllArgsConstructor
public class DefaultRepresentation extends ColumnRepresentation {

Column value;
private static final DefaultRepresentation EMPTY_REPRESENTATION = new DefaultRepresentation(
functions.lit(null));

/**
* Gets the empty representation.
*
* @return A singleton instance of an empty representation
*/
@Nonnull
public static ColumnRepresentation empty() {
return EMPTY_REPRESENTATION;
}

/**
* The column value represented by this object.
*/
private Column value;

/**
* Create a new {@link ColumnRepresentation} from a literal value.
Expand All @@ -53,7 +70,7 @@ public class DefaultRepresentation extends ColumnRepresentation {
* @return A new {@link ColumnRepresentation} representing the value
*/
@Nonnull
public static ColumnRepresentation literal(@Nonnull final Object value) {
public static DefaultRepresentation literal(@Nonnull final Object value) {
if (value instanceof BigDecimal) {
// If the literal is a BigDecimal, represent it as a DecimalRepresentation.
return new DecimalRepresentation(lit(value));
Expand Down Expand Up @@ -120,5 +137,5 @@ protected Column traverseColumn(@Nonnull final String fieldName) {
public ColumnRepresentation getField(@Nonnull final String fieldName) {
return copyOf(value.getField(fieldName));
}

}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package au.csiro.pathling.fhirpath.execution;

import au.csiro.pathling.fhirpath.collection.ResourceCollection;
import au.csiro.pathling.fhirpath.column.EmptyRepresentation;
import au.csiro.pathling.fhirpath.column.DefaultRepresentation;
import au.csiro.pathling.fhirpath.function.registry.StaticFunctionRegistry;
import ca.uhn.fhir.context.FhirContext;
import jakarta.annotation.Nonnull;
Expand All @@ -26,7 +26,7 @@ static class NullResourceResolver extends BaseResourceResolver {
@Nonnull
protected ResourceCollection createResource(@Nonnull final ResourceType resourceType) {
return ResourceCollection.build(
new EmptyRepresentation(),
DefaultRepresentation.empty(),
getFhirContext(), resourceType);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public static BooleanCollection exists(@Nonnull final Collection input,
@FhirPathFunction
@Nonnull
public static BooleanCollection empty(@Nonnull final Collection input) {
return BooleanCollection.build(input.getColumn().empty());
return BooleanCollection.build(input.getColumn().isEmpty());
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import au.csiro.pathling.fhirpath.collection.StringCollection;
import au.csiro.pathling.fhirpath.column.ColumnRepresentation;
import au.csiro.pathling.fhirpath.column.DefaultRepresentation;
import au.csiro.pathling.fhirpath.column.EmptyRepresentation;
import au.csiro.pathling.fhirpath.definition.ElementDefinition;
import au.csiro.pathling.fhirpath.function.FhirPathFunction;
import au.csiro.pathling.sql.udf.PropertyUdf;
Expand Down Expand Up @@ -71,7 +70,7 @@ public static StringCollection display(@Nonnull final CodingCollection input,
.transformWithUdf("display", Optional.ofNullable(language)
.map(StringCollection::getColumn)
.map(ColumnRepresentation::singular)
.orElse(EmptyRepresentation.getInstance()))
.orElse(DefaultRepresentation.empty()))
.removeNulls()
);
}
Expand Down Expand Up @@ -122,7 +121,7 @@ public static Collection property(@Nonnull final CodingCollection input,
Optional.ofNullable(language)
.map(StringCollection::getColumn)
.map(ColumnRepresentation::singular)
.orElse(EmptyRepresentation.getInstance())
.orElse(DefaultRepresentation.empty())
).flatten().removeNulls();

return Collection.build(resultCtx, propertyType,
Expand Down Expand Up @@ -160,11 +159,11 @@ public static StringCollection designation(@Nonnull final CodingCollection input
Optional.ofNullable(use)
.map(CodingCollection::getColumn)
.map(ColumnRepresentation::singular)
.orElse(EmptyRepresentation.getInstance()),
.orElse(DefaultRepresentation.empty()),
Optional.ofNullable(language)
.map(StringCollection::getColumn)
.map(ColumnRepresentation::singular)
.orElse(EmptyRepresentation.getInstance())
.orElse(DefaultRepresentation.empty())
)
.flatten().removeNulls()
);
Expand Down Expand Up @@ -279,7 +278,7 @@ public static CodingCollection translate(@Nonnull final Concepts input,
.transform(c -> functions.split(c, ",")),
Optional.ofNullable(target).map(StringCollection::getColumn)
.map(ColumnRepresentation::singular)
.orElse(EmptyRepresentation.getInstance())
.orElse(DefaultRepresentation.empty())
));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import au.csiro.pathling.fhirpath.FhirPath;
import au.csiro.pathling.fhirpath.collection.Collection;
import au.csiro.pathling.fhirpath.column.DefaultRepresentation;
import au.csiro.pathling.fhirpath.column.EmptyRepresentation;
import jakarta.annotation.Nonnull;
import java.util.List;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -71,7 +70,7 @@ public ProjectionResult evaluate(@Nonnull final ProjectionContext context) {

// This is a way to evaluate the expression for the purpose of getting the types of the result.
final ProjectionContext stubContext = context.withInputContext(
unnestingCollection.map(__ -> EmptyRepresentation.getInstance()));
unnestingCollection.map(__ -> DefaultRepresentation.empty()));
final List<ProjectionResult> stubResults = components.stream()
.map(s -> s.evaluate(stubContext))
.toList();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,11 +176,11 @@ void testNot() {
@Test
void testEmpty() {
new ColumnAsserts()
.assertEquals(true, nullValue().empty())
.assertEquals(false, valueOf(17).empty())
.assertEquals(true, nullArray().empty())
.assertEquals(true, emptyArray().empty())
.assertEquals(false, arrayOf(1, 2, 3).empty())
.assertEquals(true, nullValue().isEmpty())
.assertEquals(false, valueOf(17).isEmpty())
.assertEquals(true, nullArray().isEmpty())
.assertEquals(true, emptyArray().isEmpty())
.assertEquals(false, arrayOf(1, 2, 3).isEmpty())
.check();
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package au.csiro.pathling.fhirpath.yaml;

import au.csiro.pathling.test.yaml.YamlConfig;
import au.csiro.pathling.test.yaml.YamlSpec;
import au.csiro.pathling.test.yaml.YamlSpecTestBase;
import jakarta.annotation.Nonnull;
Expand All @@ -9,6 +10,9 @@

@Slf4j
@Tag("UnitTest")
@YamlConfig(
resourceBase = "fhirpath-ptl/resources"
)
public class YamlFhirpathTest extends YamlSpecTestBase {

@YamlSpec("fhirpath-ptl/cases/literals.yaml")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@

@Slf4j
@Tag("YamlTest")
@YamlConfig("fhirpath-js/config.yaml")
@YamlConfig(
config = "fhirpath-js/config.yaml",
resourceBase = "fhirpath-js/resources"
)
public class YamlReferenceImplTest extends YamlSpecCachedTestBase {


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,7 @@
@Retention(RetentionPolicy.RUNTIME)
public @interface YamlConfig {

String value();
String config() default "";

String resourceBase() default "";
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import ca.uhn.fhir.parser.IParser;
import jakarta.annotation.Nonnull;
import jakarta.annotation.Nullable;
import java.io.File;
import java.math.BigDecimal;
import java.math.RoundingMode;
import java.util.List;
Expand Down Expand Up @@ -342,8 +343,17 @@ public Stream<? extends Arguments> provideArguments(ExtensionContext context) {

final Optional<String> testConfigPath = context.getTestClass()
.flatMap(c -> Optional.ofNullable(c.getAnnotation(YamlConfig.class)))
.map(YamlConfig::value);
.map(YamlConfig::config)
.filter(s -> !s.isBlank());

final Optional<String> resourceBase = context.getTestClass()
.flatMap(c -> Optional.ofNullable(c.getAnnotation(YamlConfig.class)))
.map(YamlConfig::resourceBase)
.filter(s -> !s.isBlank());

testConfigPath.ifPresent(s -> log.info("Loading test config from: {}", s));
resourceBase.ifPresent(s -> log.info("Resource base : {}", s));

final TestConfig testConfig = testConfigPath
.map(TestResources::getResourceAsString)
.map(TestConfig::fromYaml)
Expand Down Expand Up @@ -376,7 +386,7 @@ public Stream<? extends Arguments> provideArguments(ExtensionContext context) {
.map(ts -> StdRuntimeCase.of(ts,
Optional.ofNullable(ts.getInputFile())
.map(f -> (Function<RuntimeContext, ResourceResolver>) FhirResolverFactory.of(
getResourceAsString("fhirpath-js/resources/" + f)))
getResourceAsString(resourceBase.orElse("") + File.separator + f)))
.orElse(defaultResolverFactory),
excluder.apply(ts)
))
Expand Down
Loading
Loading