-
Notifications
You must be signed in to change notification settings - Fork 96
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
Fix to use physical name instead of logical name to retrieve available interval #226
Changes from 7 commits
d8d2659
dbe991c
91abe7f
d094ec8
ee87d8a
52598ac
8b0c5e6
a451011
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ | |
import com.yahoo.bard.webservice.data.time.ZonedTimeGrain; | ||
import com.yahoo.bard.webservice.table.availability.Availability; | ||
import com.yahoo.bard.webservice.table.resolver.DataSourceConstraint; | ||
import com.yahoo.bard.webservice.table.resolver.PhysicalDataSourceConstraint; | ||
import com.yahoo.bard.webservice.util.IntervalUtils; | ||
import com.yahoo.bard.webservice.util.SimplifiedIntervalList; | ||
|
||
|
@@ -16,6 +17,9 @@ | |
|
||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.Set; | ||
import java.util.function.Function; | ||
import java.util.stream.Collectors; | ||
|
||
import javax.validation.constraints.NotNull; | ||
|
||
|
@@ -79,12 +83,37 @@ public DateTime getTableAlignment() { | |
|
||
@Override | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should JavaDoc be added here to call out the filtering that happens so that only columns that are in the Schema for this table are returned? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Still needs to be addressed I think? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added comment on the interface |
||
public Map<Column, List<Interval>> getAllAvailableIntervals() { | ||
return getAvailability().getAllAvailableIntervals(); | ||
Map<String, List<Interval>> availableIntervals = getAvailability().getAllAvailableIntervals(); | ||
|
||
return getSchema().getColumns().stream() | ||
.collect(Collectors.toMap( | ||
Function.identity(), | ||
column -> availableIntervals.getOrDefault( | ||
getSchema().getPhysicalColumnName(column.getName()), | ||
new SimplifiedIntervalList() | ||
) | ||
)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this should wrap down 1 more level (ie. unwrap the |
||
} | ||
|
||
@Override | ||
public SimplifiedIntervalList getAvailableIntervals(DataSourceConstraint constraint) { | ||
return getAvailability().getAvailableIntervals(constraint); | ||
|
||
Set<String> tableColumnNames = getSchema().getColumnNames(); | ||
Set<String> invalidColumnNames = constraint.getAllColumnNames().stream() | ||
.filter(name -> !tableColumnNames.contains(name)) | ||
.collect(Collectors.toSet()); | ||
|
||
if (!constraint.getAllColumnNames().stream().allMatch(tableColumnNames::contains)) { | ||
String message = String.format( | ||
"Received invalid request requesting for columns: %s that is not available in this table: %s", | ||
invalidColumnNames.stream().collect(Collectors.joining(",")), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rather than computing Set<String> constraintColumnNames = constraint.getAllColumnNames();
// ...
constraintColumnNames.stream().filter(name -> !tableColumnNames.contains(name)).collect(Collectors.joining(",")) |
||
getName() | ||
); | ||
LOG.error(message); | ||
throw new RuntimeException(message); | ||
} | ||
|
||
return getAvailability().getAvailableIntervals(new PhysicalDataSourceConstraint(constraint, getSchema())); | ||
} | ||
|
||
@Override | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Availbility -> Availability