> On 2012-03-15 00:35:26, brian Foster wrote: > > Why can't DataSourceCatalog be extended and just modified per hypersonic > > needs?... what are the differences exactly between DataSource and Hyersonic > > versions of the Catalogs? > > Chris Mattmann wrote: > Hey Brian, great question. The reality is that this catalog was hidden > away in the File Manager test suite inside of TestDataSourceCatalog. There > was something wiggy about the DataSourceCatalog code that Hypersonic SQL > didn't like so I had to extend DataSourceCatalog for internal testing since > it made sense to use Hypersonic SQL for that (could load up the DBMS in > memory, etc.) I know that there was like 1 method that used incompatible SQL > syntax that Hypersonic SQL didn't like. Then when I overrode that one method, > I had to override like 4-5 others to inject my subclass method into the call > chain for that which is what you see in the patch. It seems that > DataSourceCatalog has since evolved and it's entirely possible that it would > support Hypersonic SQL now, so here's what I see as 2 options: > > 1. we try and simply remove Hypersonic SQL catalog period, and then see > if the TestDataSourceCatalog passes. If it does, this issue is moot and I > will close this ReviewBoard and the issue. > > 2. If #1 doesn't succeed, and suggests changes, we could: > a) flow those changes into DataSourceCatalog -- someone would have to > have time to do that :) It also could happen later; or > b) suggest that since this catalog exists and we know it works, > temporarily in 0.4 and ongoing until someone refactors or improves > DataSourceCatalog ship the HypersonicSQLFriendlyCatalog for folks who want to > use it. > > Sound right? Thoughts? > > brian Foster wrote: > i'd be cool with you checking it in for now with a TODO at the top of the > class and maybe a JIRA issue to merge the 2 at some point > > Chris Mattmann wrote: > Cool, I will try and use the DataSourceCatalog, see if it passes, and if > so, just forget this one. If it doesn't pass, I'll consider the above your +1 > and then move forward with this. > > > Chris Mattmann wrote: > Hey Brian, here was the issue, check it: > > java.sql.SQLException: Not in aggregate function or group by clause: > org.hsqldb.Expression@16ba5c7a in statement [SELECT COUNT(DISTINCT > p.product_id) AS numResults FROM GenericFile_metadata p ORDER BY p.product_id > DESC ] > at org.hsqldb.jdbc.Util.sqlException(Unknown Source) > at org.hsqldb.jdbc.jdbcStatement.fetchResult(Unknown Source) > at org.hsqldb.jdbc.jdbcStatement.executeQuery(Unknown Source) > > Hypersonic SQL doesn't like COUNT functions and ORDER BY to be used > together. See: http://www.dcm4che.org/jira/browse/ARR-37 > > So, what I did with this catalog is override the getResultListSize, which > then caused me to have to override paginateQuery, and then caused me to have > to intercept the pagedQuery and then the full paging API, which is what this > catalog does, and I think correctly. If I don't hear any further objections, > I am going to commit this tomorrow. > > Ricky Nguyen wrote: > Why can't we remove the ORDER clause? > > SELECT COUNT(DISTINCT p.product_id) AS numResults FROM > GenericFile_metadata p > > since the order doesn't matter when you're just returning a count. > > Chris Mattmann wrote: > What do you guys think? I'd be OK removing the ORDER clause but it will > be a change from the SQL that the DataSourceCatalog used to generate. But, > you're right, it doesn't really matter. Wonder why Oracle, and MySQL and > Postgres allow it? > > brian Foster wrote: > I vote ditch ORDER! > > Chris Mattmann wrote: > Hmm VERY interesting. So, I tried to ditch ORDER, and just use > DataSourceCatalog. Here's what I got: > > [chipotle:oodt/trunk/filemgr] mattmann% more > target/surefire-reports/org.apache.oodt.cas.filemgr.catalog.TestDataSourceCatalog.txt > > > ------------------------------------------------------------------------------- > Test set: org.apache.oodt.cas.filemgr.catalog.TestDataSourceCatalog > > ------------------------------------------------------------------------------- > Tests run: 7, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.828 sec > <<< FAILURE! > > testFirstProductOnlyOnFirstPage(org.apache.oodt.cas.filemgr.catalog.TestDataSourceCatalog) > Time elapsed: 0.124 sec <<< FAILURE! > junit.framework.ComparisonFailure: expected:<[ShouldBeFirstForPage.tx]t> > but was:<[tes]t> > at junit.framework.Assert.assertEquals(Assert.java:81) > at junit.framework.Assert.assertEquals(Assert.java:87) > at > org.apache.oodt.cas.filemgr.catalog.TestDataSourceCatalog.testFirstProductOnlyOnFirstPage(TestDataSourceCatalog.java:278) > > [chipotle:oodt/trunk/filemgr] mattmann% > > I need to look more, but something is wiggy here.
When i ditch DataSourceCatalog line number 1793: getProductSql += "ORDER BY p.product_id DESC "; TestDataSourceCatalog pass for me - brian ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4358/#review5978 ----------------------------------------------------------- On 2012-03-14 23:40:55, Chris Mattmann wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4358/ > ----------------------------------------------------------- > > (Updated 2012-03-14 23:40:55) > > > Review request for oodt, brian Foster, Ricky Nguyen, Paul Ramirez, and Thomas > Bennett. > > > Summary > ------- > > DataSourceCatalog compatible with HypersonicSQL > > > This addresses bug OODT-410. > https://issues.apache.org/jira/browse/OODT-410 > > > Diffs > ----- > > > ./trunk/filemgr/src/main/java/org/apache/oodt/cas/filemgr/catalog/HsqlDbFriendlyDataSourceCatalog.java > PRE-CREATION > > ./trunk/filemgr/src/main/java/org/apache/oodt/cas/filemgr/catalog/HsqlDbFriendlyDataSourceCatalogFactory.java > PRE-CREATION > > ./trunk/filemgr/src/test/org/apache/oodt/cas/filemgr/catalog/TestDataSourceCatalog.java > 1294690 > > Diff: https://reviews.apache.org/r/4358/diff > > > Testing > ------- > > > Thanks, > > Chris > >