Skip to content

IRSA-975: NED searching seems not to work #489

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

Merged
merged 3 commits into from
Nov 2, 2017

Conversation

loitly
Copy link
Contributor

@loitly loitly commented Nov 1, 2017

https://caltech-ipac.atlassian.net/browse/IRSA-975

  • column names must be enclosed in double-quotes
  • remove caching done at the search processor’s level
  • introduce SharedDbProcessor: results from same processor share the same database
  • convert IBE’s template into a search processor
  • convert XYGenericProcessor into an EmbeddedDbProcessor
  • use firefly’s config file where applicable.
  • limit log message to 2k characters

To Test:

  • build firefly
  • Catalogs -> NED -> m16 -> 1.2 deg
    Should get a tri-view with decimated chart and image plot.
    What caused the problem was the column names. They contains spaces, parentheses, etc
    This ticket should fix all of that. There is no restricting on column name now.

- column names must be enclose by double-quotes
- remove caching done at the search processor’s level
- introduce SharedDbProcessor: results from same processor share the same database
- convert IBE’s template into a search processor
- convert XYGenericProcessor into an EmbeddedDbProcessor
- use firefly’s config file where applicable.
- limit log message to 2k characters
@loitly loitly requested review from ejoliet and tgoldina November 1, 2017 17:41
Copy link
Contributor

@tgoldina tgoldina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test case works as described.

I noticed that our expression library does not parse NED columns like "RA(dec)" or "No." Also the field descriptions from VOTable did not make into the column tooltips. These are the issues to be addressed in the future.

I have loaded a NED catalog. Then I loaded WISE to check if the tooltips are meaningful. The tooltips are present in WISE. However, when switching back to NED tab, I've got the following error:

Failed to fetch tracefalse data: Error: BadSqlGrammarException:StatementCallback; bad SQL grammar [select cname from DATA_DD]; nested exception is java.sql.SQLSyntaxErrorException: user lacks privilege or object not found: DATA_DD from:null

@loitly
Copy link
Contributor Author

loitly commented Nov 2, 2017

@tgoldina , I've just made another commit to fix the problem you found. It involved FireflyHeatMap.js returning a bad mappings object on re-mount. Can you take a look to see if the change is acceptable? Thanks.

Copy link
Contributor

@tgoldina tgoldina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works great.

@@ -28,7 +28,7 @@
* @version $Id: DbInstance.java,v 1.3 2012/03/15 20:35:40 loi Exp $
*/
abstract public class BaseDbAdapter implements DbAdapter {
private static long MAX_IDLE_TIME = 1000 * 60 * 5; // will be cleaned up if idle more than 5 minutes.
private static long MAX_IDLE_TIME = 1000 * 60 * 15; // will be purged up if idle more than 5 minutes.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the comment here does not match the number any more

@@ -114,7 +114,7 @@ function handleCatalogUpdate(tbl_id) {
const params= {
startIdx : 0,
pageSize : MAX_ROW,
inclCols : `${columns.lonCol},${columns.latCol},ROW_IDX`
inclCols : `"${columns.lonCol}","${columns.latCol}",ROW_IDX` // column names should be in quotes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only the unknown columns should be in quotes. The columns that we know (like ROW_IDX) are safe, right?

From tis point of view we did not have to add quotes to columns like Peak, Period, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, all column names should be in quotes. But because ROW_IDX is all-caps single word, it does not need to be in quotes. But, I should quotes them as well for consistency.

@loitly loitly merged commit df7a50c into dev Nov 2, 2017
@loitly loitly deleted the IRSA-975_NED_vosearch_dup_columns branch November 2, 2017 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants