Github user mvhlong commented on the pull request:
https://github.com/apache/tajo/pull/200#issuecomment-59471438
Hi @hyunsik and @jihoonson,
I have just made an important update to my patch: change from using Double
to Datum.
In the source code, I throw Exceptions for non-numeric type because I do
not want to mix too many changes at once and we need to update several
important functions in Datum before other types can be supported. However, with
this change, the histogram module can be extended to support other data types
easily in the future.
With the use of Datum, in contrast to my assumption, the histogram
construction time is not slower in the tests. This is good for us.
With a single histogram idea (for example, equi-width), I prefer to make
only 1 implementation for all data types rather than to make 3 different
versions for Long, Double, and byte[]. This makes the source code clean and
easy for maintenance. Anyway, with the latest update, this problem has been
solved.
Currently, the histogram implementation takes a list of Datum values. You
are right that it takes all values at a time. Building a histogram in an
completely incremental way is difficult because many histograms require the
sort of all data points. So, I think that we should build the histograms in a
*partially* incremental way. More specifically, we first build many histograms
with many different samples, then *merge* them. A function to merge the
histogram will be implemented later.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
