[GitHub] incubator-quickstep issue #32: QUICKSTEP-21 Measure execution time of WorkOr...

2016-06-15 Thread hbdeshmukh
Github user hbdeshmukh commented on the issue: https://github.com/apache/incubator-quickstep/pull/32 Hi @pateljm @zuyu I have added the execution time field in rebuild work order completion proto message. There are detailed comments about the rationale for splitting the orig

[GitHub] incubator-quickstep pull request #34: Bug fixed in \analyze command and reus...

2016-06-15 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/incubator-quickstep/pull/34 --- 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 fea

Re: [jira] [Commented] (QUICKSTEP-20) Add parser support for SQL window aggregation function

2016-06-15 Thread Julian Hyde
If you’re going to build a full database, go for it, best of luck to you, but you’re going to need a large community. But in my opinion, you should focus on your strengths. The community should ask itself what is the one thing that it can do better than anyone else. If you build library that ca

[GitHub] incubator-quickstep issue #34: Bug fixed in \analyze command and reuse code.

2016-06-15 Thread jianqiao
Github user jianqiao commented on the issue: https://github.com/apache/incubator-quickstep/pull/34 LGTM! Merging. --- 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 wish

[GitHub] incubator-quickstep pull request #35: Added Parser support for SQL Window Ag...

2016-06-15 Thread shixuan-fan
GitHub user shixuan-fan opened a pull request: https://github.com/apache/incubator-quickstep/pull/35 Added Parser support for SQL Window Aggregation Function Added parser support for SQL Window Aggregation Function. The parser will now understand the window aggregation funct

[GitHub] incubator-quickstep issue #32: QUICKSTEP-21 Measure execution time of normal...

2016-06-15 Thread zuyu
Github user zuyu commented on the issue: https://github.com/apache/incubator-quickstep/pull/32 @pateljm I agree that if we would go with the split option, we do w/ extensions as following. ``` message WorkOrderCompletionMessage { ... } message NormalWork

[GitHub] incubator-quickstep issue #32: QUICKSTEP-21 Measure execution time of normal...

2016-06-15 Thread hbdeshmukh
Github user hbdeshmukh commented on the issue: https://github.com/apache/incubator-quickstep/pull/32 Hi @pateljm @zuyu I can get the execution time for RebuildWorkOrder in the same PR. Thanks for your comments. --- If your project is set up for it, you can reply to this em

Re: Guideline To Update Copyright Header

2016-06-15 Thread Jignesh Patel
Hi Julian, Good point. To clarify. I was only going to copy the copyright into the root NOTICE file, and leave the files completely untouched. So, they wouldn’t have the Apache header in the source files. Only our files would have the new Apache Copyright template. Also, I think if it makes s

[GitHub] incubator-quickstep issue #34: Bug fixed in \analyze command and reuse code.

2016-06-15 Thread hbdeshmukh
Github user hbdeshmukh commented on the issue: https://github.com/apache/incubator-quickstep/pull/34 Assigning @jianqiao --- 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

[GitHub] incubator-quickstep pull request #34: Bug fixed in \analyze command and reus...

2016-06-15 Thread hbdeshmukh
GitHub user hbdeshmukh opened a pull request: https://github.com/apache/incubator-quickstep/pull/34 Bug fixed in \analyze command and reuse code. The ``\analyze`` command issues SQL queries. Due to a recent change in the execution engine (PR #14), there was a bug in issuing the quer

[GitHub] incubator-quickstep issue #32: QUICKSTEP-21 Measure execution time of normal...

2016-06-15 Thread pateljm
Github user pateljm commented on the issue: https://github.com/apache/incubator-quickstep/pull/32 @zuyu @hbdeshmukh Ok -- looking at the code, I do think that having two protos make sense. Down the line `RebuildWorkOrderCompletionMessage` may need to report back compression ratio and/

Re: Guideline To Update Copyright Header

2016-06-15 Thread Julian Hyde
If the code in third_party is merely “copied” into the project and not part of it, and in particular if have not forked it and intend to copy in a more recent version in the future, maybe you shouldn’t be applying Apache headers to those files. They could be just included in the source distribut

Re: Do we need 3 different InsertDestination classes?

2016-06-15 Thread Jignesh Patel
A huge +1 from me to get rid of dead code! +CC: Gerald who also worked on this a while back, in case he has some input. Thanks! Jignesh > On Jun 14, 2016, at 11:36 PM, Navneet Potti wrote: > > Hi Harshad > I’m just kicking off this discussion based on a conversation I had with > Jignesh th

[GitHub] incubator-quickstep issue #5: DO NOT MERGE: CI w/ gRPC To Test the Distribut...

2016-06-15 Thread zuyu
Github user zuyu commented on the issue: https://github.com/apache/incubator-quickstep/pull/5 @navsan I think it is related to how `tmb` explicitly specifies the library `gflags_nothreads-static`. And disabling the shared build works using `gcc`, although there are two cases that were

[GitHub] incubator-quickstep issue #33: BugFix: Update NumQueuedWorkOrders to fix sch...

2016-06-15 Thread zuyu
Github user zuyu commented on the issue: https://github.com/apache/incubator-quickstep/pull/33 @hbdeshmukh is right about `WorkerDirectory`, and enforcing `NUMA-aware` scheduling is the main reason to introduce this class. In the distributed version, `Foreman` needs to query t

[GitHub] incubator-quickstep issue #32: QUICKSTEP-21 Measure execution time of normal...

2016-06-15 Thread zuyu
Github user zuyu commented on the issue: https://github.com/apache/incubator-quickstep/pull/32 @pateljm Separating into two proto messages is overkill to me. In fact, the most part in this PR is to duplicate the logics to process two identical proto messages, except for `normal` and `

[GitHub] incubator-quickstep issue #32: QUICKSTEP-21 Measure execution time of normal...

2016-06-15 Thread pateljm
Github user pateljm commented on the issue: https://github.com/apache/incubator-quickstep/pull/32 @hbdeshmukh Can you rebase this again? I can then merge. Thanks! --- 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 pr

Re: [jira] [Commented] (QUICKSTEP-20) Add parser support for SQL window aggregation function

2016-06-15 Thread Jignesh Patel
Great points Julian, especially about algebra. Couldn’t agree more. In fact, we have been strong advocates of the viewpoint that it is all about the algebraic framework. Furthermore, we have argued that the relational algebraic framework is the right “core” to build a platform. With it you can g

[GitHub] incubator-quickstep pull request #33: BugFix: Update NumQueuedWorkOrders to ...

2016-06-15 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/incubator-quickstep/pull/33 --- 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 fea

Re: Do we need 3 different InsertDestination classes?

2016-06-15 Thread Harshad Deshmukh
Hi Navneet, I was always in favor of getting rid of AlwaysCreateBlock derived class. This issue was raised long time back, but Craig insisted that we keep it. I guess the PartitionAware class can't be an efficient drop in replacement for the BlockPool class. The reason is that it checks for

[GitHub] incubator-quickstep issue #32: QUICKSTEP-21 Measure execution time of normal...

2016-06-15 Thread pateljm
Github user pateljm commented on the issue: https://github.com/apache/incubator-quickstep/pull/32 Good discussion guys. If I could weigh in, I think keeping the two proto classes makes sense. I'd like to close this later on today if the discussion here has converged. Thanks! --- If

[GitHub] incubator-quickstep issue #33: BugFix: Update NumQueuedWorkOrders to fix sch...

2016-06-15 Thread pateljm
Github user pateljm commented on the issue: https://github.com/apache/incubator-quickstep/pull/33 This also points out that we should have a better CI in place for performance testing. We periodically run benchmark tests, but that is not in a CI pipeline. Travis is not ideal for this

[GitHub] incubator-quickstep issue #33: BugFix: Update NumQueuedWorkOrders to fix sch...

2016-06-15 Thread pateljm
Github user pateljm commented on the issue: https://github.com/apache/incubator-quickstep/pull/33 @navsan Nice job!! Thanks @rogersjeffreyl for helping. @hbdeshmukh Re. your question about checking the TMB, I think you only need to check the TMB periodically (i.e. for a new b