Skip to content
This repository has been archived by the owner on Jun 7, 2021. It is now read-only.

Trafodion 3084 support to create comment on hbase table #1597

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

gdgy
Copy link

@gdgy gdgy commented Jun 9, 2018

No description provided.

@Traf-Jenkins
Copy link

Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/2734/

@Traf-Jenkins
Copy link

@sandhyasun
Copy link
Contributor

@anoopsharma00 Does this look ok ? Looks ok to me and can merge with your +1.

@anoopsharma00
Copy link
Contributor

Were testcases added to regressions to test this feature?
Would this work for hbase "ROW", "CELL", "MAP" formats? They all have HBASE as catalog name.
Would 'drop hbase table' command remove added comments?
Would unregister command remove comments? It will remove the added objectuid for that hbase table.
It will also be better to use 'if not registered' instead of 'if not exists' construct. They both are supported.

@Traf-Jenkins
Copy link

Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/2761/

@Traf-Jenkins
Copy link

@Traf-Jenkins
Copy link

Previous Test Aborted. New Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/2762/

@Traf-Jenkins
Copy link

@Traf-Jenkins
Copy link

New Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/2769/

@Traf-Jenkins
Copy link

@Traf-Jenkins
Copy link

New Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/2773/

@Traf-Jenkins
Copy link

@Traf-Jenkins
Copy link

New Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/2779/

@Traf-Jenkins
Copy link

@gdgy gdgy changed the title Trafodion 3084 Trafodion 3084 support to create comment on hbase table Jun 27, 2018
Copy link
Contributor

@DaveBirdsall DaveBirdsall left a comment

Choose a reason for hiding this comment

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

+1 Looks good to me. @anoopsharma00, does this look good to you now?

@Traf-Jenkins
Copy link

Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/2880/

@Traf-Jenkins
Copy link

@Traf-Jenkins
Copy link

Can one of the admins verify this patch?

@moscowgentalman
Copy link
Contributor

jenkins, retest

@Traf-Jenkins
Copy link

Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/2951/

@Traf-Jenkins
Copy link

@Traf-Jenkins
Copy link

Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/3203/

@Traf-Jenkins
Copy link


Lng32 cliRC = cliInterface.executeImmediate(ddl.data());
// error code -8102 means the object already register
if (cliRC < 0 && cliRC != -8102)
Copy link
Contributor

Choose a reason for hiding this comment

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

Since the ddl issued is 'register ...if not registered', an 8102 error will
not be returned if object already exists.
This check should not be done and all errors should be returned.

retcode = deleteFromTextTable(&cliInterface,
objUID,
ComTextType::COM_OBJECT_COMMENT_TEXT,
0);
Copy link
Contributor

Choose a reason for hiding this comment

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

if retcode is < 0, it should return -1 indicating an error.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants