fix use of schema as reader option + add a test to seek within reader #70
Conversation
| @@ -108,7 +110,9 @@ func (c *ReaderConfig) Apply(options ...ReaderOption) { | |||
|
|
|||
| // ConfigureReader applies configuration options from c to config. | |||
| func (c *ReaderConfig) ConfigureReader(config *ReaderConfig) { | |||
There was a problem hiding this comment.
This feels like sort of a weird API now to be honest. Maybe
func (config *ReaderConfig) Clone() *ReaderConfig {
}
would be a better API?
There was a problem hiding this comment.
I'm not sure if you mean the ConfigureReader API is weird or the implementation of the method. Could you give a bit more details about what we should change?
There was a problem hiding this comment.
To give a bit more color to this, ConfigureReader is meant to implement the ReaderOption interface.
There was a problem hiding this comment.
Sorry just the whole semantics of already having a ReaderConfig, and then passing a ReaderConfig (which already has some configuration) to that ReaderConfig to be modified, just seems weird.
What about, I don't know,
// Coalesce modifies the child ReaderConfig using the schema present in parent
func (parent *ReaderConfig) Coalesce (child *ReaderConfig) {
}
or
// ConfigFromSchema coalesces the schema in parent and the schema in child, returning a ReaderConfig
// with the coalesced schema
func (parent *ReaderConfig) ConfigFromSchema (child *Schema) *ReaderConfig {
}
or something?
| @@ -615,7 +616,7 @@ func (r *filePages) SeekToRow(rowIndex int64) (err error) { | |||
| if index < 0 { | |||
| return ErrSeekOutOfRange | |||
| } | |||
| _, err = r.section.Seek(pages[index].Offset-r.dataOffset, io.SeekStart) | |||
| _, err = r.section.Seek(pages[index].Offset-r.baseOffset, io.SeekStart) | |||
There was a problem hiding this comment.
The offset was miscalculated when a dictionary page was present since the base isn't the data page offset but the dictionary page offset in that case.
Investigate and fix reported issues.