Modified according to mike's comments

yahoo · Apr 7, 2017 · 8b6212e · 8b6212e
1 parent cbbfbcd
commit 8b6212e
Show file tree

Hide file tree

Showing 7 changed files with 71 additions and 14 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -69,8 +69,8 @@ Current
 - [Fix to use physical name instead of logical name to retrieve available interval](https://github.com/yahoo/fili/pull/226)
     * `getAllAvailbleIntervals` in `ConcreteAvailability` no longer filters out unconfigured columns, instead table's `getAllAvailbleIntervals` does
     * `getAvailbleIntervals` in `Availbality` now takes `PhysicalDataSourceConstraint` instead of `DataSourceConstraint`
-    * `Availbility` no longer takes a set of columns on the table, only table needs to know
-    * `getAllAvailbleIntervals` in `Availbility` now returns a map of column physical name to interval list instead of column to interval list
+    * `Availability` no longer takes a set of columns on the table, only table needs to know
+    * `getAllAvailbleIntervals` in `Availability` now returns a map of column physical name to interval list instead of column to interval list
     * `TestDataSourceMetadataService` now takes map from string to list of intervals instead of column to list of intervals for constructor
 
 - [Allow GranularityComparator to return static instance](https://github.com/yahoo/fili/pull/225)

diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/table/BaseSchema.java b/fili-core/src/main/java/com/yahoo/bard/webservice/table/BaseSchema.java
@@ -8,6 +8,7 @@
 
 import java.util.LinkedHashSet;
 import java.util.Objects;
+import java.util.stream.Collectors;
 
 /**
  * A parent class for most schema implementations.
@@ -33,6 +34,17 @@ public LinkedHashSet<Column> getColumns() {
         return columns;
     }
 
+    /**
+     * Get the names of the columns returned by getColumns method.
+     *
+     * @return linked hash set of column names in this schema
+     */
+    public LinkedHashSet<String> getColumnNames() {
+        return getColumns().stream()
+                .map(Column::getName)
+                .collect(Collectors.toCollection(LinkedHashSet::new));
+    }
+
     @Override
     public Granularity getGranularity() {
         return granularity;

diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/table/PermissiveConcretePhysicalTable.java b/fili-core/src/main/java/com/yahoo/bard/webservice/table/PermissiveConcretePhysicalTable.java
@@ -6,6 +6,9 @@
 import com.yahoo.bard.webservice.data.time.ZonedTimeGrain;
 import com.yahoo.bard.webservice.metadata.DataSourceMetadataService;
 import com.yahoo.bard.webservice.table.availability.PermissiveAvailability;
+import com.yahoo.bard.webservice.table.resolver.DataSourceConstraint;
+import com.yahoo.bard.webservice.table.resolver.PhysicalDataSourceConstraint;
+import com.yahoo.bard.webservice.util.SimplifiedIntervalList;
 
 import java.util.Map;
 import java.util.Set;
@@ -47,4 +50,12 @@ public PermissiveConcretePhysicalTable(
                 new PermissiveAvailability(name, dataSourceMetadataService)
         );
     }
+
+    @Override
+    public SimplifiedIntervalList getAvailableIntervals(DataSourceConstraint constraint) {
+        return getAvailability().getAvailableIntervals(PhysicalDataSourceConstraint.buildWithSchemaUnion(
+                constraint,
+                getSchema()
+        ));
+    }
 }
diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/table/PhysicalTableSchema.java b/fili-core/src/main/java/com/yahoo/bard/webservice/table/PhysicalTableSchema.java
@@ -14,7 +14,7 @@
 /**
  * The schema for a physical table.
  */
-public class PhysicalTableSchema extends BaseSchema implements Schema {
+public class PhysicalTableSchema extends BaseSchema {
 
     private final ZonedTimeGrain timeGrain;
     private final Map<String, String> logicalToPhysicalColumnNames;

diff --git a/...re/src/main/java/com/yahoo/bard/webservice/table/availability/PermissiveAvailability.java b/...re/src/main/java/com/yahoo/bard/webservice/table/availability/PermissiveAvailability.java
@@ -40,8 +40,9 @@ public PermissiveAvailability(
      * <p>
      * This is different from its parent's
      * {@link
-     * com.yahoo.bard.webservice.table.availability.ConcreteAvailability#getAvailableIntervals
-     * (PhysicalDataSourceConstraint)};
+     * com.yahoo.bard.webservice.table.availability.ConcreteAvailability#getAvailableIntervals(
+     * PhysicalDataSourceConstraint
+     * )};
      * Instead of returning the intersection of all available intervals, this method returns the union of them.
      *
      * @param constraint  Data constraint containing columns and api filters. Constrains are ignored, because

diff --git a/.../src/main/java/com/yahoo/bard/webservice/table/resolver/PhysicalDataSourceConstraint.java b/.../src/main/java/com/yahoo/bard/webservice/table/resolver/PhysicalDataSourceConstraint.java
@@ -27,12 +27,31 @@ public PhysicalDataSourceConstraint(
     ) {
         super(dataSourceConstraint);
 
+        Set<String> schemaColumnNames = physicalTableSchema.getColumnNames();
+
+        this.allColumnPhysicalNames = dataSourceConstraint.getAllColumnNames().stream()
+                .filter(schemaColumnNames::contains)
+                .map(physicalTableSchema::getPhysicalColumnName)
+                .collect(Collectors.toSet());
+    }
+
+    /**
+     * Private Constructor that union columns in constraint and schema, use buildWithSchemaUnion to invoke.
+     *
+     * @param physicalTableSchema  A map from logical column name to physical column names
+     * @param dataSourceConstraint  Data source constraint containing all the column names as logical names
+     */
+    private PhysicalDataSourceConstraint(
+            PhysicalTableSchema physicalTableSchema,
+            DataSourceConstraint dataSourceConstraint
+    ) {
+        super(dataSourceConstraint);
+
         Set<String> schemaColumnNames = physicalTableSchema.getColumns().stream()
                 .map(Column::getName)
                 .collect(Collectors.toSet());
 
-        this.allColumnPhysicalNames = dataSourceConstraint.getAllColumnNames().stream()
-                .filter(schemaColumnNames::contains)
+        this.allColumnPhysicalNames = schemaColumnNames.stream()
                 .map(physicalTableSchema::getPhysicalColumnName)
                 .collect(Collectors.toSet());
     }
@@ -45,4 +64,19 @@ public PhysicalDataSourceConstraint(
     public Set<String> getAllColumnPhysicalNames() {
         return allColumnPhysicalNames;
     }
+
+    /**
+     * Builds a physical data source constraint with the union of columns in schema.
+     *
+     * @param dataSourceConstraint  Data source constraint containing all the column names as logical names
+     * @param physicalTableSchema  A map from logical column name to physical column names
+     *
+     * @return a new physical data source constraint with all columns in schema
+     */
+    public static PhysicalDataSourceConstraint buildWithSchemaUnion(
+            DataSourceConstraint dataSourceConstraint,
+            PhysicalTableSchema physicalTableSchema
+    ) {
+        return new PhysicalDataSourceConstraint(physicalTableSchema, dataSourceConstraint);
+    }
 }
diff --git a/...rc/test/groovy/com/yahoo/bard/webservice/table/PermissiveConcretePhysicalTableSpec.groovy b/...rc/test/groovy/com/yahoo/bard/webservice/table/PermissiveConcretePhysicalTableSpec.groovy
@@ -74,12 +74,11 @@ class PermissiveConcretePhysicalTableSpec extends Specification {
 
         where:
         allColumnNames                                                | expected
-        ["disjointIntervalColumn"]                                    | [disjointInterval] as Set
-        ["leftAbuttingIntervalColumn"]                                | [leftAbuttingInterval] as Set
-        ["rightAbuttingIntervalColumn"]                               | [rightAbuttingInterval] as Set
-        ["disjointIntervalColumn", "leftAbuttingIntervalColumn"]      | [disjointInterval, leftAbuttingInterval] as Set
-        ["disjointIntervalColumn",  "rightAbuttingIntervalColumn"]    | [disjointInterval, rightAbuttingInterval] as Set
-        ["leftAbuttingIntervalColumn", "rightAbuttingIntervalColumn"] | [leftAbuttingInterval, rightAbuttingInterval] as Set
-
+        ["disjointIntervalColumn"]                                    | [disjointInterval, leftAbuttingInterval, rightAbuttingInterval] as Set
+        ["leftAbuttingIntervalColumn"]                                | [disjointInterval, leftAbuttingInterval, rightAbuttingInterval] as Set
+        ["rightAbuttingIntervalColumn"]                               | [disjointInterval, leftAbuttingInterval, rightAbuttingInterval] as Set
+        ["disjointIntervalColumn", "leftAbuttingIntervalColumn"]      | [disjointInterval, leftAbuttingInterval, rightAbuttingInterval] as Set
+        ["disjointIntervalColumn",  "rightAbuttingIntervalColumn"]    | [disjointInterval, leftAbuttingInterval, rightAbuttingInterval] as Set
+        ["leftAbuttingIntervalColumn", "rightAbuttingIntervalColumn"] | [disjointInterval, leftAbuttingInterval, rightAbuttingInterval] as Set
     }
 }