-
Notifications
You must be signed in to change notification settings - Fork 16
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
Conversation
- 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
There was a problem hiding this 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
@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. |
There was a problem hiding this 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. |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
https://caltech-ipac.atlassian.net/browse/IRSA-975
To Test:
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.