Bug fix cannot set comment on native hbase table #1579
Conversation
gdgy
changed the title
|
Can one of the admins verify this patch? |
3 similar comments
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
jenkins, add user |
|
If you add "[TRAFODION-3084]" to the beginning of your subject line, then the pull request will automatically be noted in the JIRA. |
|
Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/2672/ |
DaveBirdsall
left a comment
DaveBirdsall
left a comment
There was a problem hiding this comment.
Congratulations on making your first contribution! I have several small requests for changes.
core/sql/bin/SqlciErrors.txt
Outdated
| 1718 ZZZZZ 99999 BEGINNER MINOR DBADMIN --- unused --- | ||
| 1719 ZZZZZ 99999 BEGINNER MINOR DBADMIN Access Type '$0~string0' is not supported. | ||
| 1720 ZZZZZ 99999 BEGINNER MINOR DBADMIN Isolation Level '$0~string0' is not supported. | ||
| 1722 ZZZZZ 99999 BEGINNER MINOR DBADMIN cannot set comment on '$0~string0' if the target table is native hbase table. |
There was a problem hiding this comment.
Grammar nit: The first word in the message should be capitalized. Also, "hbase" should be spelled "HBase". (Yes, it is odd by English standards, I know, but that's the way the HBase people spell it.)
There was a problem hiding this comment.
One more comment. Please add this new message to the Trafodion Messages Guide. You can find the source for that in the trafodion/docs/messages_guide directory.
| *CmpCommon::diags() << DgSqlCode(-1722); | ||
| processReturn(); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Three comments:
- There is a literal, HBASE_SYSTEM_CATALOG, that should be used instead of hard-coding the string "HBASE" in line 343. (See common/ComSmallDefs.h.)
- In your error message text, you have a $0 ~ String0 parameter, but you don't set that here. To set it, you need to add "*CmpCommon::diags() << DgString0(some string text)". Alternatively, you could remove the '$0 ~ String0' from your message text in bin/SqlciErrors.txt.
- Minor point. The indentation is irregular. Perhaps you have tabs on some lines and spaces on others. If you could make it uniform with the surrounding code it will be easier to read.
|
Test Passed. https://jenkins.esgyn.com/job/Check-PR-master/2672/ |
|
New Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/2675/ |
|
Test Failed. https://jenkins.esgyn.com/job/Check-PR-master/2675/ |
|
jenkins, retest |
|
New Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/2695/ |
core/sql/bin/SqlciErrors.txt
Outdated
| 1718 ZZZZZ 99999 BEGINNER MINOR DBADMIN --- unused --- | ||
| 1719 ZZZZZ 99999 BEGINNER MINOR DBADMIN Access Type '$0~string0' is not supported. | ||
| 1720 ZZZZZ 99999 BEGINNER MINOR DBADMIN Isolation Level '$0~string0' is not supported. | ||
| 1722 ZZZZZ 99999 BEGINNER MINOR DBADMIN cannot set comment on native HBASE table. |
There was a problem hiding this comment.
First word should be capitalized. Also, HBASE should be HBase.
Also, please add this message to the Trafodion Messages Guide. The source can be found in the trafodion/docs/messages_guide directory.
|
Test Passed. https://jenkins.esgyn.com/job/Check-PR-master/2695/ |
|
New Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/2699/ |
|
Test Failed. https://jenkins.esgyn.com/job/Check-PR-master/2699/ |
|
Previous Test Aborted. New Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/2700/ |
|
Test Failed. https://jenkins.esgyn.com/job/Check-PR-master/2700/ |
|
Previous Test Aborted. New Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/2701/ |
|
Test Failed. https://jenkins.esgyn.com/job/Check-PR-master/2701/ |
|
Previous Test Aborted. New Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/2702/ |
|
Test Passed. https://jenkins.esgyn.com/job/Check-PR-master/2702/ |
3 participants
No description provided.